-
Notifications
You must be signed in to change notification settings - Fork 207
Support BigQuery STRUCT schema change #1385
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
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for your pull request! We could not find a changelog entry for this change in the dbt-adapters package. For details on how to document a change, see the Contributing Guide. |
| {% else %} | ||
|
|
||
| {% set schema_changes_dict = check_for_schema_changes(source_relation, target_relation) %} | ||
| {% set schema_changes_dict = adapter.dispatch('check_for_schema_changes', 'dbt')(source_relation, target_relation) %} |
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.
let's update check_for_schema_changes to follow our standard pattern where it contains the dispatch logic + a default macro
E In dispatch: No macro named 'check_for_schema_changes' found within namespace: 'dbt'
E Searched for: 'test.redshift__check_for_schema_changes', 'test.postgres__check_for_schema_changes', 'test.default__check_for_schema_changes', 'dbt.redshift__check_for_schema_changes', 'dbt.postgres__check_for_schema_changes', 'dbt.default__check_for_schema_changes'
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.
Oups! I didn't check other adapters 😱
Will do 👍
| if drop_candidates: | ||
| relation_name = relation.render() | ||
| for column in drop_candidates: | ||
| drop_sql = f"ALTER TABLE {relation_name} DROP COLUMN {self.quote(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.
- Should we do this via a macro? (not really sure)
- Can we group these drops into a single call? i.e. can we do this:
ALTER TABLE `project.dataset.table_name` DROP COLUMN column_name_1, DROP COLUMN column_name_2, DROP COLUMN column_name_3;
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.
- Well that's a design decision, where else would you do it?
- Right, that's also possible to do it according to https://cloud.google.com/bigquery/docs/reference/standard-sql/data-definition-language#alter_table_drop_column_statement
| logger.debug( | ||
| 'Dropping column `{}` from table "{}".'.format(column.name, relation_name) | ||
| ) | ||
| client.query(drop_sql).result() |
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.
instead of calling client.query we should probably use the execute method so we use a consistent retry / exception handling logic
resolves #599
Problem
Previously, dbt-bigquery did not support schema changes for STRUCT (nested) fields in incremental models. When new fields were added to STRUCT columns or existing fields were removed, the table schema was not updated before incremental materialization operations (like MERGE), leading to potential data inconsistencies or failures.
Solution
This PR implements STRUCT field synchronization for BigQuery incremental models by:
on_schema_change.sqlto properly useadapter.dispatchfor adapter-specific overrides@availabledecorator to thesync_struct_columnsmethod inimpl.pyto expose it to Jinja templateson_schema_change.sqlmacro to callsync_struct_columnsduring schema change processingThe implementation supports
append_new_columnsmode fully, whilesync_all_columnsmode is skipped with appropriate warnings due to BigQuery API constraints (because of the unsupported nested column removal).Functional tests have been added to cover the functionality.
Checklist