Add a new schema test: relationships_where #161
Conversation
There was a problem hiding this comment.
This is really slick @MartinGuindon! I left a couple of pretty minor comments here around the documentation + default kwarg values, but otherwise, this PR is in really good shape!
README.md
Outdated
| models: | ||
| - name: model_name | ||
| columns: | ||
| - name: from_column |
There was a problem hiding this comment.
cool test! I'd change from_column to id here, then remove the from: argument to the relationships_where test. I think that will reflect a more typical usage of a test like this.
It's really great that you included the from argument here as well -- that makes it possible to define a test that does not apply to a particular column, eg. a test on an expression
There was a problem hiding this comment.
Good point. Done.
| {% macro test_relationships_where(model, to, field) %} | ||
|
|
||
| {% set column_name = kwargs.get('column_name', kwargs.get('from')) %} | ||
| {% set from_condition = kwargs.get('from_condition', True) %} |
There was a problem hiding this comment.
this is pretty minor, but I'd use "true" (a string) as the default value for from_condition and to_condition here
There was a problem hiding this comment.
Great feedback. Thanks, Drew
|
hey @MartinGuindon can you pull |
|
Thanks @drewbanin. Done that, and fixed a small mistake at the same time. |
| to: ref('data_test_relationships_where_table_1') | ||
| field: id | ||
| from_condition: id <> '4' |
There was a problem hiding this comment.
I think this test is failing on BigQuery because the id column is being loaded as an integer. Can you change '4' to 4 here? I think we should be good to go after that!
There was a problem hiding this comment.
Oops. Bad habit from Snowflake where 4 = '4'...
Fixed!
There was a problem hiding this comment.
Thanks for your contribution @MartinGuindon! This is ready to roll.
I'm going to merge this and cut a new dbt-utils release later today :)
Hello dbt-utils maintainers!
I'm submitting this PR to add a new schema test that has proven to be extremely useful.
Note that I'm not the original author of the macro. It was shared on Slack last May and I had preserved a copy of it in my project. I unfortunately didn't write down who was the original contributor... so my apologies to him/her.
The need for this macro came up a few times on Slack over these past few months so I thought I'd issue a PR anyway.
To this unknown personThanks to @tconbeer who shared it originally on Slack! <3