Repository: TobikoData/sqlmesh
Pull request #2440 (2024-04-15 00:36:00)

Fix: Always use CASCADE when deleting views in the physical layer for postgres and redshift
We always want a cascade delete for physical objects, as only other physical objects within SQLMesh can refer to it at the time of deletion.

When it comes to deleting views in the virtual layer, we still want to do this in a non-cascade fashion to prevent the accidental removal of views created outside SQLMesh that happen to reference a SQLMesh view model.

Reported in https://tobiko-data.slack.com/archives/C044BRE5W4S/p1712997673915609


Patch:
diff --git a/docs/guides/tablediff.md b/docs/guides/tablediff.md
index ff2bc144..f3550aab 100644
--- a/docs/guides/tablediff.md
+++ b/docs/guides/tablediff.md
@@ -29,21 +29,21 @@ For example, we could make two modifications to the [SQLMesh quickstart](../quic
 MODEL (
     name sqlmesh_example.incremental_model,
     kind INCREMENTAL_BY_TIME_RANGE (
-        time_column ds
+        time_column event_date
     ),
     start '2020-01-01',
     cron '@daily',
-    grain (id, ds)
+    grain (id, event_date)
 );
 
 SELECT
     id,
     CASE WHEN item_id = 3 THEN 4 ELSE item_id END as item_id, -- Change item_id 3 to 4
-    ds,
+    event_date,
 FROM
     sqlmesh_example.seed_model
 WHERE
-    ds between @start_ds and @end_ds
+    event_date between @start_ds and @end_ds
     AND id != 1 -- Remove row whose item_id is 1
 ```
 
diff --git a/sqlmesh/core/engine_adapter/base.py b/sqlmesh/core/engine_adapter/base.py
index 814e5876..feb1d09a 100644
--- a/sqlmesh/core/engine_adapter/base.py
+++ b/sqlmesh/core/engine_adapter/base.py
@@ -946,7 +946,7 @@ class EngineAdapter:
         view_name: TableName,
         ignore_if_not_exists: bool = True,
         materialized: bool = False,
-        cascade: bool = False,
+        **kwargs: t.Any,
     ) -> None:
         """Drop a view."""
         self.execute(
@@ -954,8 +954,8 @@ class EngineAdapter:
                 this=exp.to_table(view_name),
                 exists=ignore_if_not_exists,
                 materialized=materialized and self.SUPPORTS_MATERIALIZED_VIEWS,
-                cascade=cascade,
                 kind="VIEW",
+                **kwargs,
             )
         )
 
diff --git a/sqlmesh/core/engine_adapter/base_postgres.py b/sqlmesh/core/engine_adapter/base_postgres.py
index 049f84ee..4ca5627f 100644
--- a/sqlmesh/core/engine_adapter/base_postgres.py
+++ b/sqlmesh/core/engine_adapter/base_postgres.py
@@ -93,7 +93,7 @@ class BasePostgresEngineAdapter(EngineAdapter):
         """
         with self.transaction():
             if replace:
-                self.drop_view(view_name, materialized=materialized, cascade=True)
+                self.drop_view(view_name, materialized=materialized)
             super().create_view(
                 view_name,
                 query_or_df,
@@ -105,6 +105,21 @@ class BasePostgresEngineAdapter(EngineAdapter):
                 **create_kwargs,
             )
 
+    def drop_view(
+        self,
+        view_name: TableName,
+        ignore_if_not_exists: bool = True,
+        materialized: bool = False,
+        **kwargs: t.Any,
+    ) -> None:
+        kwargs["cascade"] = kwargs.get("cascade", True)
+        return super().drop_view(
+            view_name,
+            ignore_if_not_exists=ignore_if_not_exists,
+            materialized=materialized,
+            **kwargs,
+        )
+
     def _get_data_objects(
         self, schema_name: SchemaName, object_names: t.Optional[t.Set[str]] = None
     ) -> t.List[DataObject]:
diff --git a/sqlmesh/core/snapshot/evaluator.py b/sqlmesh/core/snapshot/evaluator.py
index ec036d6d..0e68ed05 100644
--- a/sqlmesh/core/snapshot/evaluator.py
+++ b/sqlmesh/core/snapshot/evaluator.py
@@ -1047,7 +1047,7 @@ class EmbeddedStrategy(SymbolicStrategy):
     ) -> None:
         target_name = view_name.for_environment(environment_naming_info)
         logger.info("Dropping view '%s' for non-materialized table", target_name)
-        self.adapter.drop_view(target_name)
+        self.adapter.drop_view(target_name, cascade=False)
 
 
 class PromotableStrategy(EvaluationStrategy):
@@ -1075,7 +1075,7 @@ class PromotableStrategy(EvaluationStrategy):
     ) -> None:
         target_name = view_name.for_environment(environment_naming_info)
         logger.info("Dropping view '%s'", target_name)
-        self.adapter.drop_view(target_name)
+        self.adapter.drop_view(target_name, cascade=False)
 
 
 class MaterializableStrategy(PromotableStrategy):


Test patch:
diff --git a/tests/core/engine_adapter/test_base_postgres.py b/tests/core/engine_adapter/test_base_postgres.py
index 6b1258c4..a7a9369f 100644
--- a/tests/core/engine_adapter/test_base_postgres.py
+++ b/tests/core/engine_adapter/test_base_postgres.py
@@ -50,3 +50,23 @@ def test_create_view(make_mocked_engine_adapter: t.Callable):
             call('CREATE VIEW "db"."view" AS SELECT 1'),
         ]
     )
+
+
+def test_drop_view(make_mocked_engine_adapter: t.Callable):
+    adapter = make_mocked_engine_adapter(BasePostgresEngineAdapter)
+    adapter.SUPPORTS_MATERIALIZED_VIEWS = True
+
+    adapter.drop_view("db.view")
+    adapter.drop_view("db.view", materialized=True)
+    adapter.drop_view("db.view", cascade=False)
+
+    adapter.cursor.execute.assert_has_calls(
+        [
+            # 1st call
+            call('DROP VIEW IF EXISTS "db"."view" CASCADE'),
+            # 2nd call
+            call('DROP MATERIALIZED VIEW IF EXISTS "db"."view" CASCADE'),
+            # 3rd call
+            call('DROP VIEW IF EXISTS "db"."view"'),
+        ]
+    )
diff --git a/tests/core/test_snapshot_evaluator.py b/tests/core/test_snapshot_evaluator.py
index 6680f105..64b1cff2 100644
--- a/tests/core/test_snapshot_evaluator.py
+++ b/tests/core/test_snapshot_evaluator.py
@@ -261,6 +261,26 @@ def test_promote(mocker: MockerFixture, adapter_mock, make_snapshot):
     )
 
 
+def test_demote(mocker: MockerFixture, adapter_mock, make_snapshot):
+    evaluator = SnapshotEvaluator(adapter_mock)
+
+    model = SqlModel(
+        name="test_schema.test_model",
+        kind=ViewKind(),
+        query=parse_one("SELECT a FROM tbl"),
+    )
+
+    snapshot = make_snapshot(model)
+    snapshot.categorize_as(SnapshotChangeCategory.BREAKING)
+
+    evaluator.demote([snapshot], EnvironmentNamingInfo(name="test_env"))
+
+    adapter_mock.drop_view.assert_called_once_with(
+        "test_schema__test_env.test_model",
+        cascade=False,
+    )
+
+
 def test_promote_default_catalog(adapter_mock, make_snapshot):
     evaluator = SnapshotEvaluator(adapter_mock)