Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Patch regexp_instr for Trino, so that its compatible in dbt-expectations #21

Closed
aezomz opened this issue Feb 23, 2023 · 7 comments

Comments

@aezomz
Copy link

aezomz commented Feb 23, 2023

Patch regexp_instr for Trino, so that its compatible in dbt-expectations

Reference, of why am here.
https://github.com/calogica/dbt-expectations/issues/243

Is your feature request related to a problem? Please describe.
Lets support regex expectation for Trino on
https://github.com/calogica/dbt-expectations/blob/b21b13f020f5df2c90885d0715a2e713d1204700/macros/regex/regexp_instr.sql

Describe the solution you'd like
We can use this,
https://trino.io/docs/current/functions/regexp.html
regexp_position(string, pattern, start, occurrence) → integer

Lets group similar implementation like https://github.com/dbt-msft/tsql-utils/tree/main/macros/dbt_expectations ?

@wjhrdy
Copy link

wjhrdy commented Oct 5, 2023

instead of waiting on this to get worked on I just made a quick fork of the fork and I'm overriding the function ontop of this repo

packages.yml

packages:
  - package: dbt-labs/dbt_utils
    version: 1.1.1
  - package: calogica/dbt_expectations
    version: 0.10.0
  - package: starburstdata/trino_utils
    version: 0.6.0
  - git: https://github.com/wjhrdy/dbt-trino-utils.git
    revision: 0.2.0

dbt_project.yml

dispatch:
  - macro_namespace: dbt_utils
    search_order: ['trino_utils', 'dbt_utils']
  - macro_namespace: dbt_date
    search_order: ['trino_utils', 'dbt_date']
  - macro_namespace: metrics
    search_order: ['trino_utils', 'metrics']
  - macro_namespace: dbt_expectations
    search_order: ['trino_utils_expectations', 'dbt_expectations']

this makes expectations work

@wjhrdy
Copy link

wjhrdy commented Oct 5, 2023

as soon as this gets worked on I'll use it from this repo

@hovaesco
Copy link

hovaesco commented Oct 6, 2023

@wjhrdy contributions to dbt-trino-utils are always more than welcome! If you are willing, you can submit PR with your changes and we'll review it.

@wjhrdy wjhrdy mentioned this issue Oct 7, 2023
@wjhrdy
Copy link

wjhrdy commented Dec 13, 2023

I've found and kind of addressed another issue with trino and dbt_expectations.

The test dbt_expectations.expect_column_values_to_not_match_regex is not trino compatible because it compares to 0 instead of -1 which is the trino standard.

unfortunately, the dbt_expectation test is not written in a way that is override-able so you would have to reference my patch package which is not ideal.

trino_utils_expectations.expect_column_values_to_not_match_regex

if anyone wants to use this and is willing to risk using my package it is in the 0.2.1 tag

  - git: https://github.com/wjhrdy/dbt-trino-utils.git
    revision: 0.2.1

@damian3031
Copy link
Member

...
The test dbt_expectations.expect_column_values_to_not_match_regex is not trino compatible because it compares to 0 instead of -1 which is the trino standard.
...

Yeah, I took it into account in this PR. But instead of adjusting tests, I changed macro trino__regexp_instr behaviour to return 0 instead of -1. This ensures consistency in the dbt-expectations macro behaviour across different adapters.

@aezomz aezomz closed this as not planned Won't fix, can't repro, duplicate, stale Dec 14, 2023
@wjhrdy
Copy link

wjhrdy commented Dec 14, 2023

aah that is a good approach thanks

@damian3031
Copy link
Member

Trino is now supported directly in dbt_expectations.
relevant PR: calogica/dbt-expectations#294

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants