PR feedback

Author: erindru

sqlmesh/core/plan/builder.py

@@ -712,12 +712,6 @@ def _categorize_snapshot(
                 snapshot.categorize_as(SnapshotChangeCategory.INDIRECT_NON_BREAKING, forward_only)
         else:
             # Metadata updated.
-            if not forward_only and snapshot.is_seed:
-                # seeds with metadata updates should be categorized as forward_only to prevent unnecessary backfill
-                # backfill should only happen if the actual seed data changes, in which case it will be classed as
-                # directly modified / breaking and not forward only as this code path will not be hit
-                forward_only = True
-
             snapshot.categorize_as(SnapshotChangeCategory.METADATA, forward_only)
 
     def _get_orphaned_indirect_change_category(

tests/core/test_integration.py

@@ -10664,15 +10664,15 @@ def _run_restatement_plan(tmp_path: Path, config: Config, q: queue.Queue):
     set_console(orig_console)
 
 
-def test_seed_model_metadata_update_sets_forward_only(tmp_path: Path):
+def test_seed_model_metadata_update_does_not_trigger_backfill(tmp_path: Path):
     """
     Scenario:
         - Create a seed model; perform initial population
         - Modify the model with a metadata-only change and trigger a plan
 
     Outcome:
         - The seed model is modified (metadata-only) but this should NOT trigger backfill
-        - To prevent backfill, the categorizer needs to set forward_only on the seed
+        - There should be no missing_intervals on the plan to backfill
     """
 
     models_path = tmp_path / "models"
@@ -10703,7 +10703,6 @@ def test_seed_model_metadata_update_sets_forward_only(tmp_path: Path):
     plan = ctx.plan(auto_apply=True)
 
     original_seed_snapshot = ctx.snapshots['"memory"."test"."source_data"']
-    assert not original_seed_snapshot.forward_only
     assert plan.directly_modified == {original_seed_snapshot.snapshot_id}
     assert plan.metadata_updated == set()
     assert plan.missing_intervals
@@ -10735,9 +10734,6 @@ def test_seed_model_metadata_update_sets_forward_only(tmp_path: Path):
     assert plan.has_changes
 
     new_seed_snapshot = ctx.snapshots['"memory"."test"."source_data"']
-    assert (
-        new_seed_snapshot.forward_only
-    )  # change needs to be applied as forward-only to prevent backfill
     assert (
         new_seed_snapshot.version == original_seed_snapshot.version
     )  # should be using the same physical table

tests/core/test_plan.py

@@ -1189,7 +1189,7 @@ def test_forward_only_plan_seed_models(make_snapshot, mocker: MockerFixture):
     assert not snapshot_a_updated.is_forward_only
 
 
-def test_seed_model_metadata_change_categorized_forward_only(
+def test_seed_model_metadata_change_no_missing_intervals(
     make_snapshot: t.Callable[..., Snapshot],
 ):
     snapshot_a = make_snapshot(
@@ -1241,13 +1241,11 @@ def test_seed_model_metadata_change_categorized_forward_only(
 
     plan = PlanBuilder(context_diff).build()
     assert snapshot_a_metadata_updated.change_category == SnapshotChangeCategory.METADATA
-    assert (
-        snapshot_a_metadata_updated.is_forward_only
-    )  # needs to be forward_only to prevent backfill on a metadata change
+    assert not snapshot_a_metadata_updated.is_forward_only
+    assert not plan.missing_intervals  # plan should have no missing intervals
     assert (
         snapshot_a_metadata_updated.intervals == snapshot_a.intervals
     )  # intervals should have been copied
-    assert not plan.missing_intervals  # plan should have no missing intervals
 
 
 def test_start_inference(make_snapshot, mocker: MockerFixture):