From 7fb2eaf1613b23a9addb0709d901baace06d2482 Mon Sep 17 00:00:00 2001 From: Sreerama Yeshwanth Gowd Date: Mon, 27 Jul 2026 09:19:57 +0530 Subject: [PATCH 1/5] fix: match the whole key tuple in legacy delete+insert Signed-off-by: Sreerama Yeshwanth Gowd --- .../incremental/strategies.sql | 11 +++++- .../incremental/test_delete_insert.py | 38 ++++++++++++------- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/dbt/include/databricks/macros/materializations/incremental/strategies.sql b/dbt/include/databricks/macros/materializations/incremental/strategies.sql index 7b2c73c8d..87fac5935 100644 --- a/dbt/include/databricks/macros/materializations/incremental/strategies.sql +++ b/dbt/include/databricks/macros/materializations/incremental/strategies.sql @@ -180,10 +180,19 @@ replace on ({{ replace_on_expr }}) {%- set statements = [] -%} {#-- Build WHERE clause for DELETE statement --#} + {#-- Match the whole key tuple; per-column IN would delete unmatched key combinations --#} {%- set delete_conditions = [] -%} + {%- set target_keys = [] -%} + {%- set source_keys = [] -%} {%- for key in unique_keys -%} - {%- do delete_conditions.append(target_relation ~ '.' ~ adapter.quote(key) ~ ' IN (SELECT ' ~ adapter.quote(key) ~ ' FROM ' ~ source_relation ~ ')') -%} + {%- do target_keys.append(target_relation ~ '.' ~ adapter.quote(key)) -%} + {%- do source_keys.append(adapter.quote(key)) -%} {%- endfor -%} + {%- if unique_keys | length > 1 -%} + {%- do delete_conditions.append('(' ~ target_keys | join(', ') ~ ') IN (SELECT DISTINCT ' ~ source_keys | join(', ') ~ ' FROM ' ~ source_relation ~ ')') -%} + {%- else -%} + {%- do delete_conditions.append(target_keys[0] ~ ' IN (SELECT ' ~ source_keys[0] ~ ' FROM ' ~ source_relation ~ ')') -%} + {%- endif -%} {#-- Add incremental predicates to DELETE if specified --#} {%- if incremental_predicates is sequence and incremental_predicates is not string -%} diff --git a/tests/unit/macros/materializations/incremental/test_delete_insert.py b/tests/unit/macros/materializations/incremental/test_delete_insert.py index c2e09ebc1..40c45ff0b 100644 --- a/tests/unit/macros/materializations/incremental/test_delete_insert.py +++ b/tests/unit/macros/materializations/incremental/test_delete_insert.py @@ -153,14 +153,28 @@ def test_delete_insert_legacy_sql__non_ascii_unique_key(self, template, context) assert self.clean_sql(insert_sql).startswith("insert into target") def test_delete_insert_legacy_sql__multiple_unique_keys(self, template, context): - """Multiple unique keys are each back-quoted and ANDed in the DELETE predicate.""" + """A composite unique_key matches the whole tuple, not each column independently.""" delete_sql, _ = self.render_legacy( template, context, unique_keys=["a", "b"], target_columns=("a", "b") ) clean_delete = self.clean_sql(delete_sql) - assert "target.`a` in (select `a` from source)" in clean_delete - assert "target.`b` in (select `b` from source)" in clean_delete - assert clean_delete.count(" and ") == 1 + assert "(target.`a`, target.`b`) in (select distinct `a`, `b` from source)" in clean_delete + assert clean_delete.count(" and ") == 0 + + def test_delete_insert_legacy_sql__multiple_unique_keys_with_predicates( + self, template, context + ): + """Incremental predicates are ANDed after the tuple match, not inside it.""" + delete_sql, _ = self.render_legacy( + template, + context, + unique_keys=["a", "b"], + target_columns=("a", "b"), + incremental_predicates=["a > 1"], + ) + clean_delete = self.clean_sql(delete_sql) + assert "(target.`a`, target.`b`) in (select distinct `a`, `b` from source)" in clean_delete + assert "and a > 1" in clean_delete def test_legacy_sql_generation__single_unique_key_delete(self, template, context): """Test the DELETE SQL generation for single unique key""" @@ -181,16 +195,12 @@ def test_legacy_sql_generation__single_unique_key_delete(self, template, context def test_legacy_sql_generation__multiple_unique_keys_delete(self, template, context): """Test the DELETE SQL generation for multiple unique keys""" - expected_delete = """ - delete from target - where target.a IN (SELECT a FROM source) - and target.b IN (SELECT b FROM source) - """ - - # The macro builds conditions for each key with AND - assert "target.a in" in expected_delete.lower() - assert "target.b in" in expected_delete.lower() - assert expected_delete.lower().count(" and ") == 1 + delete_sql, _ = self.render_legacy( + template, context, unique_keys=["a", "b"], target_columns=("a", "b") + ) + clean_delete = self.clean_sql(delete_sql) + assert clean_delete.startswith("delete from target where") + assert "(target.`a`, target.`b`) in (select distinct `a`, `b` from source)" in clean_delete def test_legacy_sql_generation__with_predicates_delete(self, template, context): """Test that incremental_predicates are added to DELETE WHERE clause""" From 2ac0e136185cfe8694435fa03fe35b3b3e2cb2ea Mon Sep 17 00:00:00 2001 From: Sreerama Yeshwanth Gowd Date: Mon, 27 Jul 2026 09:21:17 +0530 Subject: [PATCH 2/5] chore: update CHANGELOG for #1612 Signed-off-by: Sreerama Yeshwanth Gowd --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cfc772852..2e88eae7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ ### Fixes +- Stop `delete+insert` with a composite `unique_key` from deleting unmatched rows on DBR below 17.1 (thanks @SreeramaYeshwanthGowd!) ([#1612](https://github.com/databricks/dbt-databricks/pull/1612) resolves [#1611](https://github.com/databricks/dbt-databricks/issues/1611)) - Allow dropping a column that has governed tags ([#1597](https://github.com/databricks/dbt-databricks/pull/1597) resolves [#1323](https://github.com/databricks/dbt-databricks/issues/1323)) - Fix view materialization incorrectly producing a no-op instead of forcing recreation when `--full-refresh` is provided alongside `view_update_via_alter: true` and `use_materialization_v2: true` ([#1456](https://github.com/databricks/dbt-databricks/pull/1456) resolves [#1404](https://github.com/databricks/dbt-databricks/issues/1404)) - Support `dbt clone` and rebuilds over an existing shallow clone ([#1592](https://github.com/databricks/dbt-databricks/pull/1592) resolves [#1165](https://github.com/databricks/dbt-databricks/issues/1165)) From 479a7bb9aca38586aa6146b2af34bec8bdec0da1 Mon Sep 17 00:00:00 2001 From: Sreerama Yeshwanth Gowd Date: Mon, 27 Jul 2026 10:48:10 +0530 Subject: [PATCH 3/5] test: add functional coverage for composite unique_key delete+insert Signed-off-by: Sreerama Yeshwanth Gowd --- .../adapter/incremental/fixtures.py | 31 +++++++++++++++++++ .../test_incremental_strategies.py | 20 ++++++++++++ 2 files changed, 51 insertions(+) diff --git a/tests/functional/adapter/incremental/fixtures.py b/tests/functional/adapter/incremental/fixtures.py index ce559bb60..2aa41337c 100644 --- a/tests/functional/adapter/incremental/fixtures.py +++ b/tests/functional/adapter/incremental/fixtures.py @@ -309,6 +309,13 @@ 3,anyway """ +delete_insert_composite_key_expected = """id,color,msg +1,blue,hello +2,red,goodbye +1,red,updated +2,blue,updated +""" + delete_insert_update_schema_expected = """id 1 2 @@ -442,6 +449,30 @@ {% endif %} """ +delete_insert_composite_key_model = """ +{{ config( + materialized = 'incremental', + unique_key = ['id', 'color'], + incremental_strategy = 'delete+insert', +) }} + +{% if not is_incremental() %} + +select cast(1 as bigint) as id, 'blue' as color, 'hello' as msg +union all +select cast(2 as bigint) as id, 'red' as color, 'goodbye' as msg + +{% else %} + +-- Neither key tuple exists in the target, so nothing should be deleted. +-- Matching each key column on its own would delete both existing rows. +select cast(1 as bigint) as id, 'red' as color, 'updated' as msg +union all +select cast(2 as bigint) as id, 'blue' as color, 'updated' as msg + +{% endif %} +""" + delete_insert_with_predicates_model = """ {{ config( materialized = 'incremental', diff --git a/tests/functional/adapter/incremental/test_incremental_strategies.py b/tests/functional/adapter/incremental/test_incremental_strategies.py index 238bd3166..de861bbc3 100644 --- a/tests/functional/adapter/incremental/test_incremental_strategies.py +++ b/tests/functional/adapter/incremental/test_incremental_strategies.py @@ -451,6 +451,26 @@ def test_incremental(self, project): ) +class TestDeleteInsertCompositeKey(IncrementalBase): + @pytest.fixture(scope="class") + def models(self): + return { + "delete_insert_model.sql": fixtures.delete_insert_composite_key_model, + } + + @pytest.fixture(scope="class") + def seeds(self): + return { + "delete_insert_expected.csv": fixtures.delete_insert_composite_key_expected, + } + + def test_incremental(self, project): + self.seed_and_run_twice() + util.check_relations_equal( + project.adapter, ["delete_insert_model", "delete_insert_expected"] + ) + + class TestDeleteInsertUpdateSchema(IncrementalBase): @pytest.fixture(scope="class") def models(self): From ff112687c9b3a28199614a89ba2ed0aaa89f6499 Mon Sep 17 00:00:00 2001 From: Sreerama Yeshwanth Gowd Date: Mon, 27 Jul 2026 13:02:42 +0530 Subject: [PATCH 4/5] test: render the real macro in the legacy delete+insert tests Signed-off-by: Sreerama Yeshwanth Gowd --- .../incremental/strategies.sql | 2 +- .../adapter/incremental/fixtures.py | 14 +++++++++++ .../test_incremental_strategies.py | 4 +++ .../incremental/test_delete_insert.py | 25 +++---------------- 4 files changed, 22 insertions(+), 23 deletions(-) diff --git a/dbt/include/databricks/macros/materializations/incremental/strategies.sql b/dbt/include/databricks/macros/materializations/incremental/strategies.sql index 87fac5935..19b68641d 100644 --- a/dbt/include/databricks/macros/materializations/incremental/strategies.sql +++ b/dbt/include/databricks/macros/materializations/incremental/strategies.sql @@ -180,7 +180,7 @@ replace on ({{ replace_on_expr }}) {%- set statements = [] -%} {#-- Build WHERE clause for DELETE statement --#} - {#-- Match the whole key tuple; per-column IN would delete unmatched key combinations --#} + {#-- Match the whole key tuple; per-column IN deletes unmatched combinations (issue #1611) --#} {%- set delete_conditions = [] -%} {%- set target_keys = [] -%} {%- set source_keys = [] -%} diff --git a/tests/functional/adapter/incremental/fixtures.py b/tests/functional/adapter/incremental/fixtures.py index 2aa41337c..daaa8bae1 100644 --- a/tests/functional/adapter/incremental/fixtures.py +++ b/tests/functional/adapter/incremental/fixtures.py @@ -449,6 +449,20 @@ {% endif %} """ +force_legacy_delete_insert_macros = """ +{% macro delete_insert_sql_impl( + source_relation, target_relation, target_columns, unique_key, incremental_predicates + ) %} + {#-- Force the DBR < 17.1 path so the legacy DELETE predicate runs on any compute --#} + {%- set keys = unique_key + if unique_key is sequence and unique_key is not string + else [unique_key] -%} + {% do return(delete_insert_legacy_sql( + source_relation, target_relation, target_columns, keys, incremental_predicates + )) %} +{% endmacro %} +""" + delete_insert_composite_key_model = """ {{ config( materialized = 'incremental', diff --git a/tests/functional/adapter/incremental/test_incremental_strategies.py b/tests/functional/adapter/incremental/test_incremental_strategies.py index de861bbc3..1104cd0ed 100644 --- a/tests/functional/adapter/incremental/test_incremental_strategies.py +++ b/tests/functional/adapter/incremental/test_incremental_strategies.py @@ -458,6 +458,10 @@ def models(self): "delete_insert_model.sql": fixtures.delete_insert_composite_key_model, } + @pytest.fixture(scope="class") + def macros(self): + return {"force_legacy_delete_insert.sql": fixtures.force_legacy_delete_insert_macros} + @pytest.fixture(scope="class") def seeds(self): return { diff --git a/tests/unit/macros/materializations/incremental/test_delete_insert.py b/tests/unit/macros/materializations/incremental/test_delete_insert.py index 40c45ff0b..f5d26a6f0 100644 --- a/tests/unit/macros/materializations/incremental/test_delete_insert.py +++ b/tests/unit/macros/materializations/incremental/test_delete_insert.py @@ -177,30 +177,11 @@ def test_delete_insert_legacy_sql__multiple_unique_keys_with_predicates( assert "and a > 1" in clean_delete def test_legacy_sql_generation__single_unique_key_delete(self, template, context): - """Test the DELETE SQL generation for single unique key""" - # We'll verify by compiling a test query that uses the same logic - # Mock adapter - context["adapter"].has_dbr_capability = lambda cap: cap == "insert_by_name" - - # Build expected DELETE manually using the same logic as the macro - expected_delete = """ - delete from target - where target.a IN (SELECT a FROM source) - """ - - # The macro builds: target.{key} IN (SELECT {key} FROM source) - # This test documents the expected SQL pattern - assert "delete from" in expected_delete.lower() - assert "target.a in (select a from source)" in expected_delete.lower() - - def test_legacy_sql_generation__multiple_unique_keys_delete(self, template, context): - """Test the DELETE SQL generation for multiple unique keys""" - delete_sql, _ = self.render_legacy( - template, context, unique_keys=["a", "b"], target_columns=("a", "b") - ) + """A single unique key keeps the original per-column predicate.""" + delete_sql, _ = self.render_legacy(template, context, unique_keys=["a"]) clean_delete = self.clean_sql(delete_sql) assert clean_delete.startswith("delete from target where") - assert "(target.`a`, target.`b`) in (select distinct `a`, `b` from source)" in clean_delete + assert "target.`a` in (select `a` from source)" in clean_delete def test_legacy_sql_generation__with_predicates_delete(self, template, context): """Test that incremental_predicates are added to DELETE WHERE clause""" From 7a8f196805bc04a42bd47e8650ecfcf90839405b Mon Sep 17 00:00:00 2001 From: Sreerama Yeshwanth Gowd Date: Wed, 5 Aug 2026 20:56:50 +0530 Subject: [PATCH 5/5] fix: correlate on the whole key tuple with EXISTS instead of a row-valued IN Signed-off-by: Sreerama Yeshwanth Gowd --- .../incremental/strategies.sql | 18 +++++++++--------- .../functional/adapter/incremental/fixtures.py | 12 ++++++++---- .../incremental/test_delete_insert.py | 18 ++++++++++++------ 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/dbt/include/databricks/macros/materializations/incremental/strategies.sql b/dbt/include/databricks/macros/materializations/incremental/strategies.sql index 19b68641d..44eba7fee 100644 --- a/dbt/include/databricks/macros/materializations/incremental/strategies.sql +++ b/dbt/include/databricks/macros/materializations/incremental/strategies.sql @@ -180,18 +180,18 @@ replace on ({{ replace_on_expr }}) {%- set statements = [] -%} {#-- Build WHERE clause for DELETE statement --#} - {#-- Match the whole key tuple; per-column IN deletes unmatched combinations (issue #1611) --#} {%- set delete_conditions = [] -%} - {%- set target_keys = [] -%} - {%- set source_keys = [] -%} - {%- for key in unique_keys -%} - {%- do target_keys.append(target_relation ~ '.' ~ adapter.quote(key)) -%} - {%- do source_keys.append(adapter.quote(key)) -%} - {%- endfor -%} {%- if unique_keys | length > 1 -%} - {%- do delete_conditions.append('(' ~ target_keys | join(', ') ~ ') IN (SELECT DISTINCT ' ~ source_keys | join(', ') ~ ' FROM ' ~ source_relation ~ ')') -%} + {#-- a row-valued IN raises DELTA_UNSUPPORTED_MULTI_COL_IN_PREDICATE; correlate on + the whole tuple instead so unmatched key combinations are not deleted (issue #1611) --#} + {%- set correlation_conditions = [] -%} + {%- for key in unique_keys -%} + {%- do correlation_conditions.append(target_relation ~ '.' ~ adapter.quote(key) ~ ' <=> ' ~ source_relation ~ '.' ~ adapter.quote(key)) -%} + {%- endfor -%} + {%- do delete_conditions.append('EXISTS (SELECT 1 FROM ' ~ source_relation ~ ' WHERE ' ~ correlation_conditions | join(' AND ') ~ ')') -%} {%- else -%} - {%- do delete_conditions.append(target_keys[0] ~ ' IN (SELECT ' ~ source_keys[0] ~ ' FROM ' ~ source_relation ~ ')') -%} + {%- set key = unique_keys[0] -%} + {%- do delete_conditions.append(target_relation ~ '.' ~ adapter.quote(key) ~ ' IN (SELECT ' ~ adapter.quote(key) ~ ' FROM ' ~ source_relation ~ ')') -%} {%- endif -%} {#-- Add incremental predicates to DELETE if specified --#} diff --git a/tests/functional/adapter/incremental/fixtures.py b/tests/functional/adapter/incremental/fixtures.py index daaa8bae1..81e1c793c 100644 --- a/tests/functional/adapter/incremental/fixtures.py +++ b/tests/functional/adapter/incremental/fixtures.py @@ -310,10 +310,10 @@ """ delete_insert_composite_key_expected = """id,color,msg -1,blue,hello -2,red,goodbye +1,blue,replaced 1,red,updated 2,blue,updated +2,red,goodbye """ delete_insert_update_schema_expected = """id @@ -478,8 +478,12 @@ {% else %} --- Neither key tuple exists in the target, so nothing should be deleted. --- Matching each key column on its own would delete both existing rows. +-- (1, blue) is an exact key match and must be replaced with its new payload. +-- (1, red) and (2, blue) only bait a per-column match; matching each column on +-- its own would wrongly delete both existing rows, so they must only insert. +-- (2, red) is absent from this run and so must survive untouched. +select cast(1 as bigint) as id, 'blue' as color, 'replaced' as msg +union all select cast(1 as bigint) as id, 'red' as color, 'updated' as msg union all select cast(2 as bigint) as id, 'blue' as color, 'updated' as msg diff --git a/tests/unit/macros/materializations/incremental/test_delete_insert.py b/tests/unit/macros/materializations/incremental/test_delete_insert.py index f5d26a6f0..81957bda1 100644 --- a/tests/unit/macros/materializations/incremental/test_delete_insert.py +++ b/tests/unit/macros/materializations/incremental/test_delete_insert.py @@ -153,18 +153,21 @@ def test_delete_insert_legacy_sql__non_ascii_unique_key(self, template, context) assert self.clean_sql(insert_sql).startswith("insert into target") def test_delete_insert_legacy_sql__multiple_unique_keys(self, template, context): - """A composite unique_key matches the whole tuple, not each column independently.""" + """A composite unique_key correlates the whole tuple, not each column independently.""" delete_sql, _ = self.render_legacy( template, context, unique_keys=["a", "b"], target_columns=("a", "b") ) clean_delete = self.clean_sql(delete_sql) - assert "(target.`a`, target.`b`) in (select distinct `a`, `b` from source)" in clean_delete - assert clean_delete.count(" and ") == 0 + assert ( + "exists (select 1 from source where target.`a` <=> source.`a`" + " and target.`b` <=> source.`b`)" + ) in clean_delete + assert clean_delete.startswith("delete from target where exists") def test_delete_insert_legacy_sql__multiple_unique_keys_with_predicates( self, template, context ): - """Incremental predicates are ANDed after the tuple match, not inside it.""" + """Incremental predicates are ANDed after the EXISTS clause, not inside it.""" delete_sql, _ = self.render_legacy( template, context, @@ -173,8 +176,11 @@ def test_delete_insert_legacy_sql__multiple_unique_keys_with_predicates( incremental_predicates=["a > 1"], ) clean_delete = self.clean_sql(delete_sql) - assert "(target.`a`, target.`b`) in (select distinct `a`, `b` from source)" in clean_delete - assert "and a > 1" in clean_delete + assert ( + "exists (select 1 from source where target.`a` <=> source.`a`" + " and target.`b` <=> source.`b`)" + ) in clean_delete + assert clean_delete.endswith("and a > 1") def test_legacy_sql_generation__single_unique_key_delete(self, template, context): """A single unique key keeps the original per-column predicate."""