fix: VIEW model with metadata-only changes in VDE=dev_only should always migrate#5384
fix: VIEW model with metadata-only changes in VDE=dev_only should always migrate#5384newtonapple wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that VIEW models with metadata-only changes in virtual environment mode dev_only will always migrate their schemas, even when they don't require backfill operations.
- Updates the
requires_schema_migration_in_prodmethod to check for symbolic models rather than materialized ones - Adds logic to conditionally skip migration for VIEWs only when they're already scheduled for recreation in Backfill stages
- Adds comprehensive test coverage for the metadata-only VIEW migration scenario
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| sqlmesh/core/snapshot/definition.py | Changes schema migration condition from materialized to non-symbolic models |
| sqlmesh/core/plan/stages.py | Adds filtering logic to preserve migration for VIEWs not scheduled for backfill recreation |
| tests/core/test_plan_stages.py | Adds new test and updates existing comment for VIEW migration behavior |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
sqlmesh/core/plan/stages.py
Outdated
| if snapshots_with_schema_migration: | ||
| missing_ids_before = {s.snapshot_id for s in missing_intervals_before_promote} | ||
| missing_ids_after = {s.snapshot_id for s in missing_intervals_after_promote} | ||
| # Only skip migrate for VIEWs that are already scheduled for recreation in Backfill. |
There was a problem hiding this comment.
No, let's not do this. The reason is that some snapshots may rely on view schema for their own migrations. So we absolutely must migrate those views. Additionally, there's already logic in place to skip view recreation during evaluation when appropriate.
| assert len(virtual_stages) == 0 | ||
|
|
||
|
|
||
| def test_build_plan_stages_virtual_environment_mode_metadata_only_changes( |
There was a problem hiding this comment.
Let's also add an integration test which involves a forward-only change to a view and check that the schema changes get reflected even though there are no evaluations.
newtonapple
force-pushed
the
ddai/fix-view-model-metadata-only-changes
branch
from
d0b31ea to
a142c5f
Compare
No description provided.