ADAP-923: Resolve issue where backup parameter was inadvertently cons…

…idered in configuration change monitoring (#657)

* changelog
* add a test demonstrating the issue
* remove backup parameter from config change monitoring

.changes/unreleased/Fixes-20231103-181357.yaml

@@ -0,0 +1,6 @@
+kind: Fixes
+body: Fix handling of `backup` parameter during config change monitoring
+time: 2023-11-03T18:13:57.401994-04:00
+custom:
+  Author: mikealfare
+  Issue: "621"

dbt/adapters/redshift/relation.py

@@ -16,7 +16,6 @@
     RedshiftMaterializedViewConfig,
     RedshiftMaterializedViewConfigChangeset,
     RedshiftAutoRefreshConfigChange,
-    RedshiftBackupConfigChange,
     RedshiftDistConfigChange,
     RedshiftSortConfigChange,
     RedshiftIncludePolicy,
@@ -93,12 +92,6 @@ def materialized_view_config_changeset(
                 context=new_materialized_view.autorefresh,
             )
 
-        if new_materialized_view.backup != existing_materialized_view.backup:
-            config_change_collection.backup = RedshiftBackupConfigChange(
-                action=RelationConfigChangeAction.alter,
-                context=new_materialized_view.backup,
-            )
-
         if new_materialized_view.dist != existing_materialized_view.dist:
             config_change_collection.dist = RedshiftDistConfigChange(
                 action=RelationConfigChangeAction.alter,

dbt/adapters/redshift/relation_configs/__init__.py

@@ -9,7 +9,6 @@
 from dbt.adapters.redshift.relation_configs.materialized_view import (
     RedshiftMaterializedViewConfig,
     RedshiftAutoRefreshConfigChange,
-    RedshiftBackupConfigChange,
     RedshiftMaterializedViewConfigChangeset,
 )
 from dbt.adapters.redshift.relation_configs.policies import (

dbt/adapters/redshift/relation_configs/materialized_view.py

@@ -1,4 +1,4 @@
-from dataclasses import dataclass
+from dataclasses import dataclass, field
 from typing import Optional, Set
 
 import agate
@@ -56,7 +56,7 @@ class RedshiftMaterializedViewConfig(RedshiftRelationConfigBase, RelationConfigV
     schema_name: str
     database_name: str
     query: str
-    backup: bool = True
+    backup: bool = field(default=True, compare=False, hash=False)
     dist: RedshiftDistConfig = RedshiftDistConfig(diststyle=RedshiftDistStyle.even)
     sort: RedshiftSortConfig = RedshiftSortConfig()
     autorefresh: bool = False
@@ -241,18 +241,8 @@ def requires_full_refresh(self) -> bool:
         return False
 
 
-@dataclass(frozen=True, eq=True, unsafe_hash=True)
-class RedshiftBackupConfigChange(RelationConfigChange):
-    context: Optional[bool] = None
-
-    @property
-    def requires_full_refresh(self) -> bool:
-        return True
-
-
 @dataclass
 class RedshiftMaterializedViewConfigChangeset:
-    backup: Optional[RedshiftBackupConfigChange] = None
     dist: Optional[RedshiftDistConfigChange] = None
     sort: Optional[RedshiftSortConfigChange] = None
     autorefresh: Optional[RedshiftAutoRefreshConfigChange] = None
@@ -262,7 +252,6 @@ def requires_full_refresh(self) -> bool:
         return any(
             {
                 self.autorefresh.requires_full_refresh if self.autorefresh else False,
-                self.backup.requires_full_refresh if self.backup else False,
                 self.dist.requires_full_refresh if self.dist else False,
                 self.sort.requires_full_refresh if self.sort else False,
             }
@@ -272,7 +261,6 @@ def requires_full_refresh(self) -> bool:
     def has_changes(self) -> bool:
         return any(
             {
-                self.backup if self.backup else False,
                 self.dist if self.dist else False,
                 self.sort if self.sort else False,
                 self.autorefresh if self.autorefresh else False,

tests/functional/adapter/materialized_view_tests/test_materialized_views.py

@@ -3,6 +3,7 @@
 import pytest
 
 from dbt.adapters.base.relation import BaseRelation
+from dbt.contracts.graph.model_config import OnConfigurationChangeOption
 
 from dbt.tests.adapter.materialized_view.basic import MaterializedViewBasic
 from dbt.tests.adapter.materialized_view.changes import (
@@ -246,7 +247,30 @@ def models(self):
     def seeds(self):
         return {"my_seed.csv": MY_SEED}
 
-    def test_running_mv_with_backup_false_succeeds(self, project):
+    @pytest.fixture(scope="class")
+    def project_config_update(self):
+        return {"models": {"on_configuration_change": OnConfigurationChangeOption.Fail.value}}
+
+    @pytest.fixture(scope="function")
+    def dbt_run_results(self, project):
         run_dbt(["seed"])
-        result = run_dbt(["run"])
-        assert result[0].node.config_call_dict["backup"] is False
+        yield run_dbt(["run", "--full-refresh"])
+
+    def test_running_mv_with_backup_false_succeeds(self, dbt_run_results):
+        assert dbt_run_results[0].node.config_call_dict["backup"] is False
+
+    def test_running_mv_with_backup_false_is_idempotent(self, project, dbt_run_results):
+        """
+        Addresses: https://github.com/dbt-labs/dbt-redshift/issues/621
+        Context:
+          - The default for `backup` is `True`
+          - We cannot query `backup` for a materialized view in Redshift at the moment
+        Premise:
+          - Set `on_configuration_change` = 'fail' (via `project_config_update`)
+          - Set `backup` = False (via `NO_BACKUP_MATERIALIZED_VIEW`)
+          - Create the materialized view (via `dbt_run_results`)
+          - Run a second time forcing the configuration change monitoring
+          - There should be no changes monitored, hence the run should be successful
+        """
+        results = run_dbt(["run"])
+        assert results[0].node.config_call_dict["backup"] is False