-
Notifications
You must be signed in to change notification settings - Fork 161
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
Fix merge fails when partition is required #986
Fix merge fails when partition is required #986
Conversation
Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA. In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR. CLA has not been signed by users: @tnk-ysk |
8651295
to
da07bff
Compare
Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA. In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR. CLA has not been signed by users: @tnk-ysk |
da07bff
to
0c17b47
Compare
I'm wondering if I should add the partition_filld parameter to get_merge_sql and modify default__get_merge_sql instead of adding bigquery__get_merge_sql. |
Great contribution @tnk-ysk!
And add that condition to the predicates so that you can combine it with https://github.com/dbt-labs/dbt-bigquery/pull/994/files? |
@github-christophe-oudar I will to fix #994 after this PR is merged. |
@tnk-ysk it looks better! |
@github-christophe-oudar Also made the macro return the predicate instead of making destructive change. |
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.
LGTM, I'll let dbt Labs team takes over the review as I don't have any rights.
resolves #792
docs dbt-labs/docs.getdbt.com/#
Problem
Trying to migrate a BQ stored procedure to DBT where the partition on the target table is required. I receive this error on the table the requires a partition:
BadRequest("
Cannot query over table 'test' without a filter over column(s) 'date' that can be used for partition elimination")
Solution
Avoid require partition filter by adding partition is null or partition is not null to the merge condition.
This condition does not change the target record.
Checklist