Fix: Surface destructive schema changes on forward-only models during dev plans (#5719)#5732
Open
brucearctor wants to merge 2 commits intoSQLMesh:mainfrom
Open
Fix: Surface destructive schema changes on forward-only models during dev plans (#5719)#5732brucearctor wants to merge 2 commits intoSQLMesh:mainfrom
brucearctor wants to merge 2 commits intoSQLMesh:mainfrom
Conversation
… dev plans (SQLMesh#5719) Extend _check_destructive_additive_changes in PlanBuilder to also inspect indirectly modified forward-only snapshots for destructive/additive schema changes. Previously, only directly modified snapshots were checked, and the evaluator-level check (MigrateSchemasStage) is skipped for dev plans. Changes: - Refactor schema diffing into reusable _check_schema_change helper - Pass indirectly_modified mapping to _check_destructive_additive_changes - Add loop to check indirectly modified forward-only snapshots - Add 5 new tests for is_dev=True scenarios (ERROR/WARN/ALLOW modes, indirect modification, --forward-only flag)
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
1 participant
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.
Summary
Fixes #5719. Destructive schema changes on forward-only models are now surfaced at plan-build time during dev plans (
sqlmesh plan dev), not just prod plans.Problem
When a forward-only model has columns removed or types narrowed,
sqlmesh plan devand PR checks would succeed, but the production deployment would fail with aDestructiveChangeErrorduringMigrateSchemasStage. This left the production environment in an unfinalized state.Root Cause
PlanBuilder._check_destructive_additive_changesalready checked for destructive schema changes at plan-build time (regardless ofis_dev), but only for directly modified snapshots. Indirectly modified forward-only models (e.g., downstream models inheriting schema changes from an upstream dependency) were not checked. The evaluator-level check inMigrateSchemasStagecatches these for prod, but that stage is skipped for dev plans.Changes
sqlmesh/core/plan/builder.py_check_schema_changehelper methodindirectly_modifiedmapping to_check_destructive_additive_changestests/core/test_plan.pyAdd 5 new tests for
is_dev=Truescenarios:test_forward_only_destructive_change_dev_plan— ERROR mode raisesPlanErrortest_forward_only_destructive_change_dev_plan_warn— WARN mode logs warningtest_forward_only_destructive_change_dev_plan_allow— ALLOW mode passes silentlytest_forward_only_indirect_destructive_change_dev_plan— indirect modification path exercisedtest_forward_only_flag_destructive_change_dev_plan—--forward-onlyflag with dev planTest Results
All 27 forward-only/destructive tests pass (5 new + 22 existing), zero regressions.