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

Update the derivation of the test view name #576

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Benjamin-Knight
Copy link

Resolves #575 Use the local MD5 macro to create a more unique hash based on the SQL of the test itself

Use the local MD5 macro to create a more unique hash based on the SQL of the test itself
Add in a random element because sometimes two tests can be completely identical as the unique work of the test is done in the jinja template itself before compilation.

See https://github.com/calogica/dbt-expectations/blob/main/macros/schema_tests/table_shape/expect_table_column_count_to_equal_other_table.sql
@cody-scott
Copy link
Collaborator

Thanks @Benjamin-Knight. Will set some time aside this week to review.

@johannes-becker-bt
Copy link

johannes-becker-bt commented Oct 31, 2024

Hi,
we're having the same problem.
I'm fine with the solution and this can be ignored if it causes too much trouble

Edit: I'm really fine with the solution, just leaving the rest of my world salad here for people with spare time. {{ local_md5(main_sql) }}_{{ range(1300, 19000) | random }} should do the trick and you've got all the variables at hand anyways

Isn't the more common approach to hash the "dbt name" of the test (which should be unique as well)? Or hash plus a random seed in case someone somehow manages to be really crazy with column and table names.
You could probably have two tests that run the same sql but yaml logic should forbid that there is the same test twice.

So one would hash relationships_table1_conflict_column1__column2__ref_table2 instead of the underlying SQL.

Cheers
Hannes

@Benjamin-Knight
Copy link
Author

@johannes-becker-bt The pull currently uses a hash and a random seed as you mention for the exact reason you noted.

Using the name of the test does sound like a better hash target, I'll have to test if DBT allows the setting of test names to be the same value, I often override the name of tests to a more user friendly version and DBT would need to ensure this is unique as well for this to work.

@johannes-becker-bt
Copy link

Only if you feel like it.
Main reason I would stick with your solution: we got main_sql available. All other solutions would need much more tinkering in different places (Unless the test name is somewhere in target or something, I'm never sure about that...)

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 this pull request may close these issues.

Tests fail due to collisions in view names
3 participants