Merged
Conversation
648e470 to
cfbf179
Compare
11c69b1 to
f7eff6f
Compare
6fa1e5c to
d71d27a
Compare
izeigerman
reviewed
Aug 20, 2025
| new_columns_to_types, | ||
| ignore_destructive=new.model.on_destructive_change.is_ignore, | ||
| alter_operations = t.cast( | ||
| t.List[TableAlterOperation], |
Collaborator
There was a problem hiding this comment.
I'm curious why do you need this cast? Isn't return type of compare_columns already a subtype?
Collaborator
Author
There was a problem hiding this comment.
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
izeigerman
reviewed
Aug 20, 2025
izeigerman
approved these changes
Aug 20, 2025
2a2706a to
efd24fe
Compare
efd24fe to
390ff2b
Compare
390ff2b to
8cf1a4b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Breaking explanation: This change is breaking if you currently use the dbt adapter and have models using
on_schema_changewhere 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 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_changeon_destructive_changeon_additive_changeTwo follow up PRs are required to reach
on_schema_changeparity:append_new_columnsignores 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_changeonly 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