Repository: TobikoData/sqlmesh
Pull request #2605 (2024-05-14 03:55:21)

Fix: Ignore non-restateable models and print out a warning instead of failing the plan
Previously SQLMesh would fail the plan if the restatement list contained a model which couldn't be restated.

When this behavior is combined with selectors, it results in a subpar experience, as the selector expression may encompass a large set of models, only a small portion of which cannot be restated.

This update changes the behavior so that an error is printed out for models that are part of the restatement list but cannot be restated instead failing the plan. It also completely ignores symbolic and seed models, as restating those is a no-op


Patch:
diff --git a/docs/guides/configuration.md b/docs/guides/configuration.md
index b3945623..848fa922 100644
--- a/docs/guides/configuration.md
+++ b/docs/guides/configuration.md
@@ -194,11 +194,11 @@ Conceptually, we can group the root level parameters into the following types. E
 
 The rest of this page provides additional detail for some of the configuration options and provides brief examples. Comprehensive lists of configuration options are at the [configuration reference page](../reference/configuration.md).
 
-### Environment schemas
+### Table/view storage locations
 
 SQLMesh creates schemas, physical tables, and views in the data warehouse/engine. Learn more about why and how SQLMesh creates schema in the ["Why does SQLMesh create schemas?" FAQ](../faq/faq.md#schema-question).
 
-The default SQLMesh behavior described in the FAQ is appropriate for most deployments, but you can override where SQLMesh creates physical tables and views with the `physical_schema_override` and `environment_suffix_target` configuration options. These options are in the [environments](../reference/configuration.md#environments) section of the configuration reference page.
+The default SQLMesh behavior described in the FAQ is appropriate for most deployments, but you can override where SQLMesh creates physical tables and views with the `physical_schema_override`, `environment_suffix_target`, and `environment_catalog_mapping` configuration options. These options are in the [environments](../reference/configuration.md#environments) section of the configuration reference page.
 
 #### Physical table schemas
 By default, SQLMesh creates physical tables for a model with a naming convention of `sqlmesh__[model schema]`.
@@ -229,7 +229,7 @@ If you had a model name of `my_schema.table`, the physical table would be create
 
 This key only applies to the _physical tables_ that SQLMesh creates - the views are still created in `my_schema` (prod) or `my_schema__<env>`.
 
-#### Environment view schemas
+#### Disable environment-specific schemas
 
 SQLMesh stores `prod` environment views in the schema in a model's name - for example, the `prod` views for a model `my_schema.users` will be located in `my_schema`.
 
@@ -306,17 +306,11 @@ With the example configuration above, SQLMesh would evaluate environment names a
 * If the environment name starts with `dev`, the catalog will be `dev`.
 * If the environment name starts with `analytics_repo`, the catalog will be `cicd`.
 
-*Note:* This feature is only available for engines that support querying across catalogs. At the time of writing, these engines are supported:
+*Note:* This feature is only available for engines that support querying across catalogs. At the time of writing, the following engines are **NOT** supported:
 
-* [BigQuery](../integrations/engines/bigquery.md)
-* [Databricks](../integrations/engines/databricks.md)
-* [DuckDB](../integrations/engines/duckdb.md)
-* [MotherDuck](../integrations/engines/motherduck.md)
 * [MySQL](../integrations/engines/mysql.md)
-* [MSSQL](../integrations/engines/mssql.md)
-* [Snowflake](../integrations/engines/snowflake.md)
-* [Spark](../integrations/engines/spark.md)
-* [Trino](../integrations/engines/trino.md)
+* [Postgres](../integrations/engines/postgres.md)
+* [GCP Postgres](../integrations/engines/gcp-postgres.md)
 
 ##### Regex Tips
 * If you are less familiar with regex, you can use a tool like [regex101](https://regex101.com/) to help you build your regex patterns.
diff --git a/mkdocs.yml b/mkdocs.yml
index 38ea36a8..4364e997 100644
--- a/mkdocs.yml
+++ b/mkdocs.yml
@@ -112,6 +112,8 @@ theme:
     - navigation.tracking
     - navigation.tabs
     - navigation.sections
+    - toc.integrate
+    - toc.follow
 plugins:
   - include-markdown
   - search
diff --git a/sqlmesh/core/context.py b/sqlmesh/core/context.py
index 5aeb0969..2f4af67a 100644
--- a/sqlmesh/core/context.py
+++ b/sqlmesh/core/context.py
@@ -1004,10 +1004,6 @@ class GenericContext(BaseContext, t.Generic[C]):
         expanded_restate_models = None
         if restate_models is not None:
             expanded_restate_models = model_selector.expand_model_selections(restate_models)
-            if not expanded_restate_models:
-                self.console.log_error(
-                    f"Provided restated models do not match any models. No models will be included in plan. Provided: {', '.join(restate_models)}"
-                )
 
         snapshots = self._snapshots(models_override)
         context_diff = self._context_diff(
@@ -1083,6 +1079,7 @@ class GenericContext(BaseContext, t.Generic[C]):
             ),
             end_bounded=not run,
             ensure_finalized_snapshots=self.config.plan.use_finalized_state,
+            console=self.console,
         )
 
     def apply(
diff --git a/sqlmesh/core/plan/builder.py b/sqlmesh/core/plan/builder.py
index e223754c..5f1bea37 100644
--- a/sqlmesh/core/plan/builder.py
+++ b/sqlmesh/core/plan/builder.py
@@ -3,10 +3,12 @@ from __future__ import annotations
 import re
 import sys
 import typing as t
+import logging
 from collections import defaultdict
 from datetime import datetime
 from functools import cached_property
 
+from sqlmesh.core.console import Console, get_console
 from sqlmesh.core.config import (
     AutoCategorizationMode,
     CategorizerConfig,
@@ -28,6 +30,9 @@ from sqlmesh.utils.date import TimeLike, now, to_datetime, yesterday_ds
 from sqlmesh.utils.errors import NoChangesPlanError, PlanError, SQLMeshError
 
 
+logger = logging.getLogger(__name__)
+
+
 class PlanBuilder:
     """Plan Builder constructs a Plan based on user choices for how they want to backfill, preview, etc. their changes.
 
@@ -88,6 +93,7 @@ class PlanBuilder:
         enable_preview: bool = False,
         end_bounded: bool = False,
         ensure_finalized_snapshots: bool = False,
+        console: t.Optional[Console] = None,
     ):
         self._context_diff = context_diff
         self._no_gaps = no_gaps
@@ -101,12 +107,13 @@ class PlanBuilder:
         self._categorizer_config = categorizer_config or CategorizerConfig()
         self._auto_categorization_enabled = auto_categorization_enabled
         self._include_unmodified = include_unmodified
-        self._restate_models = set(restate_models or [])
+        self._restate_models = set(restate_models) if restate_models is not None else None
         self._effective_from = effective_from
         self._execution_time = execution_time
         self._backfill_models = backfill_models
         self._end = end or default_end
         self._apply = apply
+        self._console = console or get_console()
 
         self._start = start
         if not self._start and self._forward_only_preview_needed:
@@ -286,9 +293,15 @@ class PlanBuilder:
                 return False
             return not snapshot.is_symbolic and not snapshot.is_seed
 
-        restatements: t.Dict[SnapshotId, Interval] = {}
-        dummy_interval = (sys.maxsize, -sys.maxsize)
         restate_models = self._restate_models
+        if restate_models == set():
+            # This is a warning but we print this as error since the Console is lacking API for warnings.
+            self._console.log_error(
+                "Provided restated models do not match any models. No models will be included in plan."
+            )
+            return {}
+
+        restatements: t.Dict[SnapshotId, Interval] = {}
         forward_only_preview_needed = self._forward_only_preview_needed
         if not restate_models and forward_only_preview_needed:
             # Add model names for new forward-only snapshots to the restatement list
@@ -298,18 +311,27 @@ class PlanBuilder:
                 for s in self._context_diff.new_snapshots.values()
                 if s.is_materialized and (self._forward_only or s.model.forward_only)
             }
+
         if not restate_models:
-            return restatements
+            return {}
 
         # Add restate snapshots and their downstream snapshots
+        dummy_interval = (sys.maxsize, -sys.maxsize)
         for model_fqn in restate_models:
             snapshot = self._model_fqn_to_snapshot.get(model_fqn)
-            if not snapshot or (
-                not forward_only_preview_needed and not is_restateable_snapshot(snapshot)
-            ):
-                raise PlanError(
-                    f"Cannot restate from '{model_fqn}'. Either such model doesn't exist, no other materialized model references it, or restatement was disabled for this model."
-                )
+            if not snapshot:
+                raise PlanError(f"Cannot restate model '{model_fqn}'. Model does not exist.")
+            if not forward_only_preview_needed:
+                if not self._is_dev and snapshot.disable_restatement:
+                    # This is a warning but we print this as error since the Console is lacking API for warnings.
+                    self._console.log_error(
+                        f"Cannot restate model '{model_fqn}'. Restatement is disabled for this model."
+                    )
+                    continue
+                elif snapshot.is_symbolic or snapshot.is_seed:
+                    logger.info("Skipping restatement for model '%s'", model_fqn)
+                    continue
+
             restatements[snapshot.snapshot_id] = dummy_interval
             for downstream_s_id in dag.downstream(snapshot.snapshot_id):
                 if is_restateable_snapshot(self._context_diff.snapshots[downstream_s_id]):
@@ -632,7 +654,9 @@ class PlanBuilder:
                 )
 
     def _ensure_no_new_snapshots_with_restatements(self) -> None:
-        if self._restate_models and self._context_diff.new_snapshots:
+        if self._restate_models is not None and (
+            self._context_diff.new_snapshots or self._context_diff.modified_snapshots
+        ):
             raise PlanError(
                 "Model changes and restatements can't be a part of the same plan. "
                 "Revert or apply changes before proceeding with restatements."


Test patch:
diff --git a/tests/core/test_plan.py b/tests/core/test_plan.py
index 62a15c03..f14748a2 100644
--- a/tests/core/test_plan.py
+++ b/tests/core/test_plan.py
@@ -357,7 +357,7 @@ def test_restate_models_with_existing_missing_intervals(sushi_context: Context):
     assert plan.requires_backfill
 
 
-def test_restate_model_with_merge_strategy(make_snapshot, mocker: MockerFixture):
+def test_restate_symbolic_model(make_snapshot, mocker: MockerFixture):
     snapshot_a = make_snapshot(
         SqlModel(
             name="a",
@@ -381,11 +381,61 @@ def test_restate_model_with_merge_strategy(make_snapshot, mocker: MockerFixture)
         previous_finalized_snapshots=None,
     )
 
+    plan = PlanBuilder(context_diff, restate_models=[snapshot_a.name]).build()
+    assert not plan.restatements
+
+
+def test_restate_seed_model(make_snapshot, mocker: MockerFixture):
+    snapshot_a = make_snapshot(
+        SeedModel(
+            name="a",
+            kind=SeedKind(path="./path/to/seed"),
+            seed=Seed(content="new_content"),
+            column_hashes={"col": "hash2"},
+            depends_on=set(),
+        )
+    )
+
+    context_diff = ContextDiff(
+        environment="test_environment",
+        is_new_environment=True,
+        is_unfinalized_environment=False,
+        create_from="prod",
+        added=set(),
+        removed_snapshots={},
+        modified_snapshots={},
+        snapshots={snapshot_a.snapshot_id: snapshot_a},
+        new_snapshots={},
+        previous_plan_id=None,
+        previously_promoted_snapshot_ids=set(),
+        previous_finalized_snapshots=None,
+    )
+
+    plan = PlanBuilder(context_diff, restate_models=[snapshot_a.name]).build()
+    assert not plan.restatements
+
+
+def test_restate_missing_model(make_snapshot, mocker: MockerFixture):
+    context_diff = ContextDiff(
+        environment="test_environment",
+        is_new_environment=True,
+        is_unfinalized_environment=False,
+        create_from="prod",
+        added=set(),
+        removed_snapshots={},
+        modified_snapshots={},
+        snapshots={},
+        new_snapshots={},
+        previous_plan_id=None,
+        previously_promoted_snapshot_ids=set(),
+        previous_finalized_snapshots=None,
+    )
+
     with pytest.raises(
         PlanError,
-        match="Cannot restate from 'a'. Either such model doesn't exist, no other materialized model references it.*",
+        match=r"Cannot restate model 'missing'. Model does not exist.",
     ):
-        PlanBuilder(context_diff, restate_models=["a"]).build()
+        PlanBuilder(context_diff, restate_models=["missing"]).build()
 
 
 def test_new_snapshots_with_restatements(make_snapshot, mocker: MockerFixture):
@@ -1111,8 +1161,8 @@ def test_disable_restatement(make_snapshot, mocker: MockerFixture):
         previous_finalized_snapshots=None,
     )
 
-    with pytest.raises(PlanError, match="""Cannot restate from '"a"'.*"""):
-        PlanBuilder(context_diff, restate_models=['"a"']).build()
+    plan = PlanBuilder(context_diff, restate_models=['"a"']).build()
+    assert not plan.restatements
 
     # Effective from doesn't apply to snapshots for which restatements are disabled.
     plan = PlanBuilder(context_diff, forward_only=True, effective_from="2023-01-01").build()