-
Notifications
You must be signed in to change notification settings - Fork 163
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
[ADAP-1044] [Bug] Incremental model with merge strategy fails to remove __dbt_tmp table when using on_schema_change=sync_all_columns #1032
Comments
Thanks for reporting this @MikkoPaajanen ! I don't see any reason why that table shouldn't be dropped after the merge. In fact, it seems to me that any intermediate tables should be dropped as a cleanup step by the end of the materialization. Acceptance criteria
Bonus criteriaEach of these is optional and ordered from the most impactful to least.
|
Implementation ideaTo do this most robustly, I'd think we'd want to refactor these lines: dbt-bigquery/dbt/include/bigquery/macros/materializations/incremental.sql Lines 154 to 156 in 0685167
i.e, remove the lines above and add the code below somewhere in here: {%- if tmp_relation_exists -%}
{{ adapter.drop_relation(tmp_relation) }}
{%- endif -%} |
Closing in favor of #1036. Please re-open this issue if you can observe the temp tables persisting for more than 12 hours. |
Re-labeling as closed in favor of #1036 (gray "closed") rather than "closed as completed / resolved" (purple "closed"). |
Is this a new bug in dbt-bigquery?
Current Behavior
Current behaviour:
adapter does not remove __dbt_tmp table when using
+on_schema_change: "sync_all_columns"
and incremental strategy "merge".I know these are created as "temp" tables but these are not actually bigquery temp tables as stated in: #163 (comment)
Expected Behavior
After running merge script it should check if following are true (tmp_relation_exists and strategy = "merge") and then drop tmp relation.
I believe this:
Would resolve the issue if added in :
dbt-bigquery/dbt/include/bigquery/macros/materializations/incremental.sql
Line 157 in 0c5422c
I don't see any reason why temp table shouldn't be dropped after the merge. Is there a reason?
Steps To Reproduce
In the model:
{{ config(materialized='incremental') }}
In dbt_project.yml:
+on_schema_change: "sync_all_columns"
Relevant log output
No response
Environment
Additional Context
I did find some discussion on the topic in other closed issues:
#163
#154
The text was updated successfully, but these errors were encountered: