-
Notifications
You must be signed in to change notification settings - Fork 134
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
Created bigquery__test_unique to handle expressions passed to BQ uniq… #10
Created bigquery__test_unique to handle expressions passed to BQ uniq… #10
Conversation
@@ -194,3 +193,24 @@ | |||
{%- endcall %} | |||
|
|||
{% endmacro %} | |||
|
|||
|
|||
{% macro bigquery__test_unique(model, column_name) %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the correct file for this macro to be added to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, this works!
It could also go into its own file, something like dbt/include/bigquery/macros/generic_tests/unique.sql
. There's no functional difference, it's just a question of organization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DigUpTheHatchet Thanks so much for the contribution! The change looks great, as does the additional test.
As far as the changelog, you can add a note under ## dbt-bigquery 1.0.0 (Release TBD)
> ### Fixes
, something like:
### Fixes
- Reimplement the `unique` test to handle column expressions and naming overlaps ([#33](https://github.com/dbt-labs/dbt-bigquery/issues/33), [#35](https://github.com/dbt-labs/dbt-bigquery/issues/35), [#10](https://github.com/dbt-labs/dbt-bigquery/pull/10))
You should also add a Contributors list to the bottom, and add yourself:
### Contributors
- [@DigUpTheHatchet](https://github.com/DigUpTheHatchet) ([#10](https://github.com/dbt-labs/dbt-bigquery/pull/10))
@@ -194,3 +193,24 @@ | |||
{%- endcall %} | |||
|
|||
{% endmacro %} | |||
|
|||
|
|||
{% macro bigquery__test_unique(model, column_name) %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, this works!
It could also go into its own file, something like dbt/include/bigquery/macros/generic_tests/unique.sql
. There's no functional difference, it's just a question of organization.
@@ -16,6 +16,9 @@ models: | |||
# this whole model should pass and run | |||
- name: table_summary | |||
description: "The summary table" | |||
tests: | |||
- unique: | |||
column_name: "concat(favorite_color_copy, count)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
description: "Testing a column with the same name as the model" | ||
tests: | ||
- not_null | ||
- unique |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going to close and re-open to trigger integration tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the integration tests failed in CI due to a change we'd made in main
that didn't pull into this branch. I ran tests/integration/schema_tests_test/
locally with success, so I'm happy to merge this PR as is.
Thanks so much for the contribution @DigUpTheHatchet!
dbt-labs#10) * Created bigquery__test_unique to handle expressions passed to BQ unique schema test * Added test on column with the same name as model & updated CHANGELOG.md
resolves #3998
This change is being made to fix a breaking change introduced in PR: 3812. Since this change was made, dbt would encounter a BQ database error when an expression (e.g. a
concat
function call) was passed as input to aunique
schema test. This testing pattern is documented here.I have not updated the CHANGELOG.md as I don't see any template/past usage of this for the dbt-bigquery repo. Could somebody advise me on this? I apologise in advance for need hand-holding!Thanks to @jtcohen6 and @Dimi727 for their investigation/analysis in the Bug Report: #3998
Checklist
[x] I have signed the CLA
[x] I have run this code in development and it appears to resolve the stated issue
[x] This PR includes tests, or tests are not required/relevant for this PR
[x] I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.