Skip to content

feat!: add on_additive_change support#5193

Merged
eakmanrq merged 1 commit intomainfrom
eakmanrq/additive_change_v3
Aug 20, 2025
Merged

feat!: add on_additive_change support#5193
eakmanrq merged 1 commit intomainfrom
eakmanrq/additive_change_v3

Conversation

@eakmanrq
Copy link
Collaborator

@eakmanrq eakmanrq commented Aug 19, 2025

Breaking explanation: This change is breaking if you currently use the dbt adapter and have models using on_schema_change where we used to not correctly mimic the behavior in the SQLMesh runtime. This is non-breaking for SQLMesh native users.

Adds the ability to control the behavior of additive changes similar to destructive changes. Has the same options as destructive.

This PR also includes two refactors to SchemaDiffer:

  • The SchemaDiffer now returns the classes it creates that contains the information related to the change instead of SQLGlot Alter expressions. This makes it easier to apply logic on top of these changes like determining the type of change and extracting the columns.
  • The SchemaDiffer now returns classes specific to each change type instead of a single class with an enum to differentiate. This allowed the ClusterByMixin to properly document it's change type and integrate with the changes from the SchemaDiffer. Also cleaned up some confusion around optional/required fields across the changes.

The dbt adapter is updated to use these changes. See table below for a mapping of behavior from dbt -> SQLMesh (the docs also contain this):

on_schema_change SQLMesh on_destructive_change SQLMesh on_additive_change
ignore ignore ignore
fail error error
append_new_columns ignore allow
sync_all_columns allow allow

Two follow up PRs are required to reach on_schema_change parity:

  • append_new_columns ignores alters even if the alters are additive. This PR would consider that additive and allow the change through. So need a way for dbt adapter to configure schema differ to consider alters destructive so it can be ignored.
  • on_schema_change only tracks top-level column changes while, on some engines, SQLMesh tracks nested column changes. Therefore there needs to be a way to let dbt adapter to configure SQLMesh so it doesn't do nested changes.

dbt documentation: https://docs.getdbt.com/docs/build/incremental-models#what-if-the-columns-of-my-incremental-model-change

@eakmanrq eakmanrq force-pushed the eakmanrq/additive_change_v3 branch 2 times, most recently from 648e470 to cfbf179 Compare August 20, 2025 01:20
@eakmanrq eakmanrq force-pushed the eakmanrq/additive_change_v3 branch 3 times, most recently from 11c69b1 to f7eff6f Compare August 20, 2025 15:39
@eakmanrq eakmanrq marked this pull request as ready for review August 20, 2025 15:44
@eakmanrq eakmanrq force-pushed the eakmanrq/additive_change_v3 branch 2 times, most recently from 6fa1e5c to d71d27a Compare August 20, 2025 16:21
new_columns_to_types,
ignore_destructive=new.model.on_destructive_change.is_ignore,
alter_operations = t.cast(
t.List[TableAlterOperation],
Copy link
Collaborator

@izeigerman izeigerman Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why do you need this cast? Isn't return type of compare_columns already a subtype?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The differ returns TableAlterColumnOperation since everything is generates are related to column operations. Methods called using this alter_operations result refer to the base class like this:

def has_drop_alteration(alter_operations: t.List[TableAlterOperation]) -> bool:

As a result I get an error like this:

sqlmesh/core/plan/builder.py:578: error: Argument 1 to "has_drop_alteration" has incompatible type "list[TableAlterColumnOperation]"; expected "list[TableAlterOperation]"  [arg-type]
sqlmesh/core/plan/builder.py:578: note: "List" is invariant -- see https://mypy.readthedocs.io/en/stable/common_issues.html#variance
sqlmesh/core/plan/builder.py:578: note: Consider using "Sequence" instead, which is covariant

@eakmanrq eakmanrq force-pushed the eakmanrq/additive_change_v3 branch 2 times, most recently from 2a2706a to efd24fe Compare August 20, 2025 21:54
@eakmanrq eakmanrq enabled auto-merge (squash) August 20, 2025 21:54
@eakmanrq eakmanrq force-pushed the eakmanrq/additive_change_v3 branch from efd24fe to 390ff2b Compare August 20, 2025 22:02
@eakmanrq eakmanrq force-pushed the eakmanrq/additive_change_v3 branch from 390ff2b to 8cf1a4b Compare August 20, 2025 22:22
@eakmanrq eakmanrq merged commit 1e2760f into main Aug 20, 2025
27 of 28 checks passed
@eakmanrq eakmanrq deleted the eakmanrq/additive_change_v3 branch August 20, 2025 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants