-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 _dbt_max_partition
declaration and initialization for BigQuery incremental models
#2976
Fix _dbt_max_partition
declaration and initialization for BigQuery incremental models
#2976
Conversation
Thanks for the PR @pcasteran!
Totally fair. We're all just trying to optimize for BigQuery's cost structure, and in this (weird) case there's a powerful feature available in legacySQL that's yet to be implemented for standardSQL.
Also reasonable. Perhaps we should add support for a new config that would override the default value used within dbt today. So, for instance:
Would you be up to open that issue? Then I'd be happy to approve this narrower PR for v0.20.0. |
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.
Would you be up to open that issue? Then I'd be happy to approve this narrower PR for v0.20.0.
Update: This issue is effectively already open in #2278. I'll move my above comment over there.
@pcasteran Could you pull the changes from develop
(new base branch), and move your changelog entry up to v0.20.0?
@jtcohen6 as requested I rebased my branch on the new |
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.
Thanks for this @pcasteran!
resolves #2940
Description
Fixed the declaration and initialization of the
_dbt_max_partition
variable for BigQuery incremental models to be able to declare additional variables.I only fixed the problem described in the issue #2940 and did not implemented the additional change you proposed @jtcohen6.
I don't really like the idea of mixing legacy sql execution, followed by client-side computation to find the latest partition and finally the actual model.
I would rather keep the declaration of the
_dbt_max_partition
variable like this and add a model option to be able to deactivate it when not needed in order to save processing time and money.Maybe it is better to open a separate issue to discuss this and keep this PR only for the variable declaration issue.
Checklist
CHANGELOG.md
and added information about my change to the "dbt next" section.