From 937c8c7d257055ed6cd3eaf7f7e5e01026d7b7d7 Mon Sep 17 00:00:00 2001
From: Quigley Malcolm <QMalcolm@users.noreply.github.com>
Date: Thu, 17 Oct 2024 20:45:23 -0700
Subject: [PATCH] Always ensure valid incremental strategy (#331)

Co-authored-by: Mike Alfare <13974384+mikealfare@users.noreply.github.com>
---
 .../unreleased/Fixes-20241016-160412.yaml     |  6 +++
 .../dbt/tests/adapter/basic/files.py          |  7 ++++
 .../tests/adapter/basic/test_incremental.py   | 39 +++++++++++++++++++
 .../models/incremental/incremental.sql        |  5 ++-
 4 files changed, 55 insertions(+), 2 deletions(-)
 create mode 100644 .changes/unreleased/Fixes-20241016-160412.yaml

diff --git a/.changes/unreleased/Fixes-20241016-160412.yaml b/.changes/unreleased/Fixes-20241016-160412.yaml
new file mode 100644
index 00000000..00a1982c
--- /dev/null
+++ b/.changes/unreleased/Fixes-20241016-160412.yaml
@@ -0,0 +1,6 @@
+kind: Fixes
+body: Always validate an incremental model's `incremental_strategy`
+time: 2024-10-16T16:04:12.58581-05:00
+custom:
+  Author: QMalcolm
+  Issue: "330"
diff --git a/dbt-tests-adapter/dbt/tests/adapter/basic/files.py b/dbt-tests-adapter/dbt/tests/adapter/basic/files.py
index 751b01a0..d0253a53 100644
--- a/dbt-tests-adapter/dbt/tests/adapter/basic/files.py
+++ b/dbt-tests-adapter/dbt/tests/adapter/basic/files.py
@@ -186,6 +186,10 @@
   {{ config(materialized="incremental") }}
 """
 
+config_materialized_incremental_invalid_strategy = """
+  {{ config(materialized="incremental", incremental_strategy="bad_strategy") }}
+"""
+
 config_materialized_var = """
   {{ config(materialized=var("materialized_var", "table"))}}
 """
@@ -217,3 +221,6 @@
 ephemeral_view_sql = config_materialized_view + model_ephemeral
 ephemeral_table_sql = config_materialized_table + model_ephemeral
 incremental_sql = config_materialized_incremental + model_incremental
+incremental_invalid_strategy_sql = (
+    config_materialized_incremental_invalid_strategy + model_incremental
+)
diff --git a/dbt-tests-adapter/dbt/tests/adapter/basic/test_incremental.py b/dbt-tests-adapter/dbt/tests/adapter/basic/test_incremental.py
index fe04a5a1..57cc4db9 100644
--- a/dbt-tests-adapter/dbt/tests/adapter/basic/test_incremental.py
+++ b/dbt-tests-adapter/dbt/tests/adapter/basic/test_incremental.py
@@ -86,6 +86,45 @@ def test_incremental_not_schema_change(self, project):
         assert run_result == RunStatus.Success
 
 
+class BaseIncrementalBadStrategy:
+    @pytest.fixture(scope="class")
+    def project_config_update(self):
+        return {"name": "incremental"}
+
+    @pytest.fixture(scope="class")
+    def models(self):
+        return {
+            "incremental.sql": files.incremental_invalid_strategy_sql,
+            "schema.yml": files.schema_base_yml,
+        }
+
+    @pytest.fixture(scope="class")
+    def seeds(self):
+        return {"base.csv": files.seeds_base_csv, "added.csv": files.seeds_added_csv}
+
+    @pytest.fixture(autouse=True)
+    def clean_up(self, project):
+        yield
+        with project.adapter.connection_named("__test"):
+            relation = project.adapter.Relation.create(
+                database=project.database, schema=project.test_schema
+            )
+            project.adapter.drop_schema(relation)
+
+    def test_incremental_invalid_strategy(self, project):
+        # seed command
+        results = run_dbt(["seed"])
+        assert len(results) == 2
+
+        # try to run the incremental model, it should fail on the first attempt
+        results = run_dbt(["run"], expect_pass=False)
+        assert len(results.results) == 1
+        assert (
+            'dbt could not find an incremental strategy macro with the name "get_incremental_bad_strategy_sql"'
+            in results.results[0].message
+        )
+
+
 class Testincremental(BaseIncremental):
     pass
 
diff --git a/dbt/include/global_project/macros/materializations/models/incremental/incremental.sql b/dbt/include/global_project/macros/materializations/models/incremental/incremental.sql
index f932751a..41d2de26 100644
--- a/dbt/include/global_project/macros/materializations/models/incremental/incremental.sql
+++ b/dbt/include/global_project/macros/materializations/models/incremental/incremental.sql
@@ -32,6 +32,9 @@
 
   {% set to_drop = [] %}
 
+  {% set incremental_strategy = config.get('incremental_strategy') or 'default' %}
+  {% set strategy_sql_macro_func = adapter.get_incremental_strategy_macro(context, incremental_strategy) %}
+
   {% if existing_relation is none %}
       {% set build_sql = get_create_table_as_sql(False, target_relation, sql) %}
   {% elif full_refresh_mode %}
@@ -52,9 +55,7 @@
     {% endif %}
 
     {#-- Get the incremental_strategy, the macro to use for the strategy, and build the sql --#}
-    {% set incremental_strategy = config.get('incremental_strategy') or 'default' %}
     {% set incremental_predicates = config.get('predicates', none) or config.get('incremental_predicates', none) %}
-    {% set strategy_sql_macro_func = adapter.get_incremental_strategy_macro(context, incremental_strategy) %}
     {% set strategy_arg_dict = ({'target_relation': target_relation, 'temp_relation': temp_relation, 'unique_key': unique_key, 'dest_columns': dest_columns, 'incremental_predicates': incremental_predicates }) %}
     {% set build_sql = strategy_sql_macro_func(strategy_arg_dict) %}