From 4ff5506e8e5b4eadb80cd5a52fc287f2531fcc44 Mon Sep 17 00:00:00 2001 From: Tomasz Zorawik <67728999+xardasos@users.noreply.github.com> Date: Mon, 17 Nov 2025 14:44:57 +0100 Subject: [PATCH 1/8] add test --- tests/core/test_snapshot_evaluator.py | 36 +++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/core/test_snapshot_evaluator.py b/tests/core/test_snapshot_evaluator.py index 9dd645ac15..acdf9cd192 100644 --- a/tests/core/test_snapshot_evaluator.py +++ b/tests/core/test_snapshot_evaluator.py @@ -24,6 +24,7 @@ DataObject, DataObjectType, InsertOverwriteStrategy, + CommentCreationView, ) from sqlmesh.core.environment import EnvironmentNamingInfo from sqlmesh.core.macros import RuntimeStage, macro, MacroEvaluator, MacroFunc @@ -1519,6 +1520,41 @@ def test_migrate_view( ) +def test_migrate_view_recreation_not_needed( + mocker: MockerFixture, + make_snapshot, + make_mocked_engine_adapter, +): + model = SqlModel( + name="test_schema.test_model", + kind=ViewKind(), + description="my_description", + query=parse_one("SELECT c, a FROM tbl"), + ) + snapshot = make_snapshot(model, version="1") + snapshot.change_category = SnapshotChangeCategory.METADATA + snapshot.forward_only = False + + adapter = make_mocked_engine_adapter(EngineAdapter) + adapter.COMMENT_CREATION_VIEW = CommentCreationView.UNSUPPORTED + adapter.with_settings = lambda **kwargs: adapter + mocker.patch( + "sqlmesh.core.engine_adapter.base.EngineAdapter.get_data_objects", + return_value=[ + DataObject( + schema="sqlmesh__test_schema", + name=f"test_schema__test_model__{snapshot.version}", + type="view", + ) + ], + ) + + evaluator = SnapshotEvaluator(adapter) + evaluator.migrate([snapshot], {}, directly_or_indirectly_modified_snapshots_ids=set()) + + adapter.cursor.execute.assert_not_called() + + def test_migrate_snapshot_data_object_type_mismatch( mocker: MockerFixture, make_snapshot, From 91b7c7b9eb8b53572e1e7ad49e9b8c71ccd0c834 Mon Sep 17 00:00:00 2001 From: Tomasz Zorawik <67728999+xardasos@users.noreply.github.com> Date: Mon, 17 Nov 2025 14:46:16 +0100 Subject: [PATCH 2/8] pass modified snapshots --- sqlmesh/core/plan/evaluator.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sqlmesh/core/plan/evaluator.py b/sqlmesh/core/plan/evaluator.py index f2f432a97e..615459d805 100644 --- a/sqlmesh/core/plan/evaluator.py +++ b/sqlmesh/core/plan/evaluator.py @@ -15,6 +15,7 @@ """ import abc +import itertools import logging import typing as t from sqlmesh.core import analytics @@ -379,6 +380,12 @@ def visit_migrate_schemas_stage( allow_destructive_snapshots=plan.allow_destructive_models, allow_additive_snapshots=plan.allow_additive_models, deployability_index=stage.deployability_index, + directly_or_indirectly_modified_snapshots_ids=set( + itertools.chain( + *plan.indirectly_modified_snapshots.values(), + plan.directly_modified_snapshots, + ) + ), ) except NodeExecutionFailedError as ex: raise PlanError(str(ex.__cause__) if ex.__cause__ else str(ex)) From 5f42366e7b429a199459087e35b346c0c2bf4762 Mon Sep 17 00:00:00 2001 From: Tomasz Zorawik <67728999+xardasos@users.noreply.github.com> Date: Mon, 17 Nov 2025 14:51:12 +0100 Subject: [PATCH 3/8] recreate is not needed for views in case of metadata changes + comments not supported --- sqlmesh/core/snapshot/evaluator.py | 39 ++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/sqlmesh/core/snapshot/evaluator.py b/sqlmesh/core/snapshot/evaluator.py index 1808011854..75097eb13f 100644 --- a/sqlmesh/core/snapshot/evaluator.py +++ b/sqlmesh/core/snapshot/evaluator.py @@ -477,6 +477,7 @@ def migrate( allow_destructive_snapshots: t.Optional[t.Set[str]] = None, allow_additive_snapshots: t.Optional[t.Set[str]] = None, deployability_index: t.Optional[DeployabilityIndex] = None, + directly_or_indirectly_modified_snapshots_ids: t.Set[SnapshotId] = None, ) -> None: """Alters a physical snapshot table to match its snapshot's schema for the given collection of snapshots. @@ -486,6 +487,7 @@ def migrate( allow_destructive_snapshots: Set of snapshots that are allowed to have destructive schema changes. allow_additive_snapshots: Set of snapshots that are allowed to have additive schema changes. deployability_index: Determines snapshots that are deployable in the context of this evaluation. + directly_or_indirectly_modified_snapshots_ids: Set of SnapshotIds with direct or indirect changes. """ deployability_index = deployability_index or DeployabilityIndex.all_deployable() target_data_objects = self._get_physical_data_objects(target_snapshots, deployability_index) @@ -510,6 +512,10 @@ def migrate( allow_additive_snapshots, self.get_adapter(s.model_gateway), deployability_index, + only_metadata_changes=s.snapshot_id + not in directly_or_indirectly_modified_snapshots_ids + if directly_or_indirectly_modified_snapshots_ids is not None + else False, ), self.ddl_concurrent_tasks, ) @@ -1111,6 +1117,7 @@ def _migrate_snapshot( allow_additive_snapshots: t.Set[str], adapter: EngineAdapter, deployability_index: DeployabilityIndex, + only_metadata_changes: bool, ) -> None: if not snapshot.is_model or snapshot.is_symbolic: return @@ -1154,6 +1161,7 @@ def _migrate_snapshot( allow_destructive_snapshots=allow_destructive_snapshots, allow_additive_snapshots=allow_additive_snapshots, run_pre_post_statements=True, + only_metadata_changes=only_metadata_changes, ) else: self._execute_create( @@ -1190,6 +1198,7 @@ def _migrate_target_table( allow_destructive_snapshots: t.Set[str], allow_additive_snapshots: t.Set[str], run_pre_post_statements: bool = False, + only_metadata_changes: bool = None, ) -> None: adapter = self.get_adapter(snapshot.model.gateway) @@ -1226,6 +1235,7 @@ def _migrate_target_table( ignore_destructive=snapshot.model.on_destructive_change.is_ignore, ignore_additive=snapshot.model.on_additive_change.is_ignore, deployability_index=deployability_index, + only_metadata_changes=only_metadata_changes, ) finally: if snapshot.is_materialized: @@ -2760,20 +2770,23 @@ def migrate( **kwargs: t.Any, ) -> None: logger.info("Migrating view '%s'", target_table_name) - model = snapshot.model - render_kwargs = dict( - execution_time=now(), snapshots=kwargs["snapshots"], engine_adapter=self.adapter - ) + if not ( + kwargs["only_metadata_changes"] and self.adapter.COMMENT_CREATION_VIEW.is_unsupported + ): + model = snapshot.model + render_kwargs = dict( + execution_time=now(), snapshots=kwargs["snapshots"], engine_adapter=self.adapter + ) - self.adapter.create_view( - target_table_name, - model.render_query_or_raise(**render_kwargs), - model.columns_to_types, - materialized=self._is_materialized_view(model), - view_properties=model.render_physical_properties(**render_kwargs), - table_description=model.description, - column_descriptions=model.column_descriptions, - ) + self.adapter.create_view( + target_table_name, + model.render_query_or_raise(**render_kwargs), + model.columns_to_types, + materialized=self._is_materialized_view(model), + view_properties=model.render_physical_properties(**render_kwargs), + table_description=model.description, + column_descriptions=model.column_descriptions, + ) # Apply grants after view migration deployability_index = kwargs.get("deployability_index") From 2fab743b5f09323988841a658cc49dadcfd2de13 Mon Sep 17 00:00:00 2001 From: Tomasz Zorawik <67728999+xardasos@users.noreply.github.com> Date: Mon, 17 Nov 2025 15:09:42 +0100 Subject: [PATCH 4/8] safe default --- sqlmesh/core/snapshot/evaluator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlmesh/core/snapshot/evaluator.py b/sqlmesh/core/snapshot/evaluator.py index 75097eb13f..9641ffcc02 100644 --- a/sqlmesh/core/snapshot/evaluator.py +++ b/sqlmesh/core/snapshot/evaluator.py @@ -1198,7 +1198,7 @@ def _migrate_target_table( allow_destructive_snapshots: t.Set[str], allow_additive_snapshots: t.Set[str], run_pre_post_statements: bool = False, - only_metadata_changes: bool = None, + only_metadata_changes: bool = False, ) -> None: adapter = self.get_adapter(snapshot.model.gateway) From 1920166dbf3e296ab9313de2aaa1a73072d17f7a Mon Sep 17 00:00:00 2001 From: Tomasz Zorawik <67728999+xardasos@users.noreply.github.com> Date: Mon, 17 Nov 2025 16:00:58 +0100 Subject: [PATCH 5/8] optional --- sqlmesh/core/snapshot/evaluator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlmesh/core/snapshot/evaluator.py b/sqlmesh/core/snapshot/evaluator.py index 9641ffcc02..98936284e0 100644 --- a/sqlmesh/core/snapshot/evaluator.py +++ b/sqlmesh/core/snapshot/evaluator.py @@ -477,7 +477,7 @@ def migrate( allow_destructive_snapshots: t.Optional[t.Set[str]] = None, allow_additive_snapshots: t.Optional[t.Set[str]] = None, deployability_index: t.Optional[DeployabilityIndex] = None, - directly_or_indirectly_modified_snapshots_ids: t.Set[SnapshotId] = None, + directly_or_indirectly_modified_snapshots_ids: t.Optional[t.Set[SnapshotId]] = None, ) -> None: """Alters a physical snapshot table to match its snapshot's schema for the given collection of snapshots. From 58b07a0ca3854f5367fb700d47f4fc5175eb259a Mon Sep 17 00:00:00 2001 From: Tomasz Zorawik <67728999+xardasos@users.noreply.github.com> Date: Tue, 25 Nov 2025 13:44:33 +0100 Subject: [PATCH 6/8] Different approach: decide whether we should recreate based on forward only and virtual env mode flags --- sqlmesh/core/plan/evaluator.py | 7 ------- sqlmesh/core/snapshot/evaluator.py | 18 ++++++------------ tests/core/test_snapshot_evaluator.py | 14 ++++++++++++-- 3 files changed, 18 insertions(+), 21 deletions(-) diff --git a/sqlmesh/core/plan/evaluator.py b/sqlmesh/core/plan/evaluator.py index 615459d805..f2f432a97e 100644 --- a/sqlmesh/core/plan/evaluator.py +++ b/sqlmesh/core/plan/evaluator.py @@ -15,7 +15,6 @@ """ import abc -import itertools import logging import typing as t from sqlmesh.core import analytics @@ -380,12 +379,6 @@ def visit_migrate_schemas_stage( allow_destructive_snapshots=plan.allow_destructive_models, allow_additive_snapshots=plan.allow_additive_models, deployability_index=stage.deployability_index, - directly_or_indirectly_modified_snapshots_ids=set( - itertools.chain( - *plan.indirectly_modified_snapshots.values(), - plan.directly_modified_snapshots, - ) - ), ) except NodeExecutionFailedError as ex: raise PlanError(str(ex.__cause__) if ex.__cause__ else str(ex)) diff --git a/sqlmesh/core/snapshot/evaluator.py b/sqlmesh/core/snapshot/evaluator.py index 98936284e0..a450daede5 100644 --- a/sqlmesh/core/snapshot/evaluator.py +++ b/sqlmesh/core/snapshot/evaluator.py @@ -477,7 +477,6 @@ def migrate( allow_destructive_snapshots: t.Optional[t.Set[str]] = None, allow_additive_snapshots: t.Optional[t.Set[str]] = None, deployability_index: t.Optional[DeployabilityIndex] = None, - directly_or_indirectly_modified_snapshots_ids: t.Optional[t.Set[SnapshotId]] = None, ) -> None: """Alters a physical snapshot table to match its snapshot's schema for the given collection of snapshots. @@ -487,7 +486,6 @@ def migrate( allow_destructive_snapshots: Set of snapshots that are allowed to have destructive schema changes. allow_additive_snapshots: Set of snapshots that are allowed to have additive schema changes. deployability_index: Determines snapshots that are deployable in the context of this evaluation. - directly_or_indirectly_modified_snapshots_ids: Set of SnapshotIds with direct or indirect changes. """ deployability_index = deployability_index or DeployabilityIndex.all_deployable() target_data_objects = self._get_physical_data_objects(target_snapshots, deployability_index) @@ -512,10 +510,6 @@ def migrate( allow_additive_snapshots, self.get_adapter(s.model_gateway), deployability_index, - only_metadata_changes=s.snapshot_id - not in directly_or_indirectly_modified_snapshots_ids - if directly_or_indirectly_modified_snapshots_ids is not None - else False, ), self.ddl_concurrent_tasks, ) @@ -1117,7 +1111,6 @@ def _migrate_snapshot( allow_additive_snapshots: t.Set[str], adapter: EngineAdapter, deployability_index: DeployabilityIndex, - only_metadata_changes: bool, ) -> None: if not snapshot.is_model or snapshot.is_symbolic: return @@ -1161,7 +1154,6 @@ def _migrate_snapshot( allow_destructive_snapshots=allow_destructive_snapshots, allow_additive_snapshots=allow_additive_snapshots, run_pre_post_statements=True, - only_metadata_changes=only_metadata_changes, ) else: self._execute_create( @@ -1198,7 +1190,6 @@ def _migrate_target_table( allow_destructive_snapshots: t.Set[str], allow_additive_snapshots: t.Set[str], run_pre_post_statements: bool = False, - only_metadata_changes: bool = False, ) -> None: adapter = self.get_adapter(snapshot.model.gateway) @@ -1235,7 +1226,6 @@ def _migrate_target_table( ignore_destructive=snapshot.model.on_destructive_change.is_ignore, ignore_additive=snapshot.model.on_additive_change.is_ignore, deployability_index=deployability_index, - only_metadata_changes=only_metadata_changes, ) finally: if snapshot.is_materialized: @@ -2770,8 +2760,12 @@ def migrate( **kwargs: t.Any, ) -> None: logger.info("Migrating view '%s'", target_table_name) - if not ( - kwargs["only_metadata_changes"] and self.adapter.COMMENT_CREATION_VIEW.is_unsupported + # Optimization: avoid unnecessary recreation when possible + if ( + snapshot.model.forward_only + or bool(snapshot.model.physical_version) + or not snapshot.virtual_environment_mode.is_full + or not self.adapter.COMMENT_CREATION_VIEW.is_unsupported ): model = snapshot.model render_kwargs = dict( diff --git a/tests/core/test_snapshot_evaluator.py b/tests/core/test_snapshot_evaluator.py index acdf9cd192..da1e10f871 100644 --- a/tests/core/test_snapshot_evaluator.py +++ b/tests/core/test_snapshot_evaluator.py @@ -1520,10 +1520,20 @@ def test_migrate_view( ) +@pytest.mark.parametrize( + "change_category", + [ + SnapshotChangeCategory.BREAKING, + SnapshotChangeCategory.NON_BREAKING, + SnapshotChangeCategory.METADATA, + ], +) +@pytest.mark.tz def test_migrate_view_recreation_not_needed( mocker: MockerFixture, make_snapshot, make_mocked_engine_adapter, + change_category: SnapshotChangeCategory, ): model = SqlModel( name="test_schema.test_model", @@ -1532,7 +1542,7 @@ def test_migrate_view_recreation_not_needed( query=parse_one("SELECT c, a FROM tbl"), ) snapshot = make_snapshot(model, version="1") - snapshot.change_category = SnapshotChangeCategory.METADATA + snapshot.change_category = change_category snapshot.forward_only = False adapter = make_mocked_engine_adapter(EngineAdapter) @@ -1550,7 +1560,7 @@ def test_migrate_view_recreation_not_needed( ) evaluator = SnapshotEvaluator(adapter) - evaluator.migrate([snapshot], {}, directly_or_indirectly_modified_snapshots_ids=set()) + evaluator.migrate([snapshot], {}) adapter.cursor.execute.assert_not_called() From 53969dbf0e5cc2eafde6b0e64fb8d55fbfb0a100 Mon Sep 17 00:00:00 2001 From: Tomasz Zorawik <67728999+xardasos@users.noreply.github.com> Date: Tue, 25 Nov 2025 21:42:55 +0100 Subject: [PATCH 7/8] fix test --- tests/core/test_snapshot_evaluator.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/core/test_snapshot_evaluator.py b/tests/core/test_snapshot_evaluator.py index da1e10f871..89a6cb1b80 100644 --- a/tests/core/test_snapshot_evaluator.py +++ b/tests/core/test_snapshot_evaluator.py @@ -1528,7 +1528,6 @@ def test_migrate_view( SnapshotChangeCategory.METADATA, ], ) -@pytest.mark.tz def test_migrate_view_recreation_not_needed( mocker: MockerFixture, make_snapshot, From ed3c1c5fafbc22c9e60cf1d7d20da614b5ca34a2 Mon Sep 17 00:00:00 2001 From: Tomasz Zorawik <67728999+xardasos@users.noreply.github.com> Date: Wed, 26 Nov 2025 10:36:28 +0100 Subject: [PATCH 8/8] fix forward-only changes + indirect changes should still cause the recreation --- sqlmesh/core/snapshot/evaluator.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sqlmesh/core/snapshot/evaluator.py b/sqlmesh/core/snapshot/evaluator.py index a450daede5..217548d850 100644 --- a/sqlmesh/core/snapshot/evaluator.py +++ b/sqlmesh/core/snapshot/evaluator.py @@ -66,6 +66,7 @@ SnapshotIdBatch, SnapshotInfoLike, SnapshotTableCleanupTask, + SnapshotChangeCategory, ) from sqlmesh.core.snapshot.execution_tracker import QueryExecutionTracker from sqlmesh.utils import random_id, CorrelationId, AttributeDict @@ -2762,9 +2763,10 @@ def migrate( logger.info("Migrating view '%s'", target_table_name) # Optimization: avoid unnecessary recreation when possible if ( - snapshot.model.forward_only + snapshot.is_forward_only or bool(snapshot.model.physical_version) or not snapshot.virtual_environment_mode.is_full + or snapshot.change_category == SnapshotChangeCategory.INDIRECT_NON_BREAKING or not self.adapter.COMMENT_CREATION_VIEW.is_unsupported ): model = snapshot.model