From 6fc7a9e69752a64567dc52305752685ac878e1e3 Mon Sep 17 00:00:00 2001 From: Amar Datar <57144500+amardatar@users.noreply.github.com> Date: Tue, 27 Feb 2024 10:54:17 +0000 Subject: [PATCH 1/6] Update SQL to allow handling null columns --- .../macros/materializations/models/incremental/merge.sql | 6 +++--- dbt/tests/adapter/incremental/test_incremental_unique_id.py | 5 +++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/dbt/include/global_project/macros/materializations/models/incremental/merge.sql b/dbt/include/global_project/macros/materializations/models/incremental/merge.sql index ca972c9f..afdb8dec 100644 --- a/dbt/include/global_project/macros/materializations/models/incremental/merge.sql +++ b/dbt/include/global_project/macros/materializations/models/incremental/merge.sql @@ -62,12 +62,12 @@ {% if unique_key %} {% if unique_key is sequence and unique_key is not string %} - delete from {{target }} + delete from {{ target }} using {{ source }} where ( {% for key in unique_key %} - {{ source }}.{{ key }} = {{ target }}.{{ key }} - {{ "and " if not loop.last}} + ({{ source }}.{{ key }} = {{ target }}.{{ key }} or ({{ source }}.{{ key }} is null and {{ target }}.{{ key }} is null)) + {{ "and " if not loop.last }} {% endfor %} {% if incremental_predicates %} {% for predicate in incremental_predicates %} diff --git a/dbt/tests/adapter/incremental/test_incremental_unique_id.py b/dbt/tests/adapter/incremental/test_incremental_unique_id.py index bddf407e..02c1923d 100644 --- a/dbt/tests/adapter/incremental/test_incremental_unique_id.py +++ b/dbt/tests/adapter/incremental/test_incremental_unique_id.py @@ -240,6 +240,8 @@ select 'NY','New York','Manhattan','2021-04-01' union all select 'PA','Philadelphia','Philadelphia','2021-05-21' +union all +select 'CO','Denver',null,'2021-06-18' """ @@ -265,6 +267,8 @@ select 'NY','New York','Manhattan','2021-04-01' union all select 'PA','Philadelphia','Philadelphia','2021-05-21' +union all +select 'CO','Devner',null,'2021-06-18' """ @@ -288,6 +292,7 @@ NY,Kings,Brooklyn,2021-04-02 NY,New York,Manhattan,2021-04-01 PA,Philadelphia,Philadelphia,2021-05-21 +CO,Denver,,2021-06-18 """ seeds__add_new_rows_sql = """ From 896861a05929cc1d026a2826dc4908e395b03a99 Mon Sep 17 00:00:00 2001 From: Amar Datar <57144500+amardatar@users.noreply.github.com> Date: Tue, 27 Feb 2024 11:01:21 +0000 Subject: [PATCH 2/6] Add changelog entry --- .changes/unreleased/Fixes-20240227105926.yaml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changes/unreleased/Fixes-20240227105926.yaml diff --git a/.changes/unreleased/Fixes-20240227105926.yaml b/.changes/unreleased/Fixes-20240227105926.yaml new file mode 100644 index 00000000..c03f6b9b --- /dev/null +++ b/.changes/unreleased/Fixes-20240227105926.yaml @@ -0,0 +1,7 @@ +kind: Fixes +body: Fix incremental merge to use an `is null` check to allow for handling of nullable + fields in the unique_key +time: 2024-02-27T10:59:59.202000+00:00 +custom: + Author: amardatar + Issue: "7597" \ No newline at end of file From e51568e01cdf6fc8d3ada61424f7c457655ae2b4 Mon Sep 17 00:00:00 2001 From: Amar Datar <57144500+amardatar@users.noreply.github.com> Date: Tue, 27 Feb 2024 11:03:26 +0000 Subject: [PATCH 3/6] Fix typo --- dbt/tests/adapter/incremental/test_incremental_unique_id.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbt/tests/adapter/incremental/test_incremental_unique_id.py b/dbt/tests/adapter/incremental/test_incremental_unique_id.py index 02c1923d..7e34a891 100644 --- a/dbt/tests/adapter/incremental/test_incremental_unique_id.py +++ b/dbt/tests/adapter/incremental/test_incremental_unique_id.py @@ -268,7 +268,7 @@ union all select 'PA','Philadelphia','Philadelphia','2021-05-21' union all -select 'CO','Devner',null,'2021-06-18' +select 'CO','Denver',null,'2021-06-18' """ From be915b029c28278ab2247706821e531c62f8c8c1 Mon Sep 17 00:00:00 2001 From: amardatar <57144500+amardatar@users.noreply.github.com> Date: Wed, 13 Mar 2024 11:09:22 +0000 Subject: [PATCH 4/6] Also handle the merge strategy --- .../macros/materializations/models/incremental/merge.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbt/include/global_project/macros/materializations/models/incremental/merge.sql b/dbt/include/global_project/macros/materializations/models/incremental/merge.sql index afdb8dec..f1be13cc 100644 --- a/dbt/include/global_project/macros/materializations/models/incremental/merge.sql +++ b/dbt/include/global_project/macros/materializations/models/incremental/merge.sql @@ -16,13 +16,13 @@ {% if unique_key is sequence and unique_key is not mapping and unique_key is not string %} {% for key in unique_key %} {% set this_key_match %} - DBT_INTERNAL_SOURCE.{{ key }} = DBT_INTERNAL_DEST.{{ key }} + DBT_INTERNAL_SOURCE.{{ key }} = DBT_INTERNAL_DEST.{{ key }} or (DBT_INTERNAL_SOURCE.{{ key }} is null and DBT_INTERNAL_DEST.{{ key }} is null) {% endset %} {% do predicates.append(this_key_match) %} {% endfor %} {% else %} {% set unique_key_match %} - DBT_INTERNAL_SOURCE.{{ unique_key }} = DBT_INTERNAL_DEST.{{ unique_key }} + DBT_INTERNAL_SOURCE.{{ unique_key }} = DBT_INTERNAL_DEST.{{ unique_key }} or (DBT_INTERNAL_SOURCE.{{ unique_key }} is null and DBT_INTERNAL_DEST.{{ unique_key }} is null) {% endset %} {% do predicates.append(unique_key_match) %} {% endif %} From f1afeddc7bd12da445291dc7bf807463709cf0fe Mon Sep 17 00:00:00 2001 From: Doug Beatty Date: Tue, 14 May 2024 11:40:55 -0600 Subject: [PATCH 5/6] pre-commit: fix end of files --- .changes/unreleased/Fixes-20240227105926.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/unreleased/Fixes-20240227105926.yaml b/.changes/unreleased/Fixes-20240227105926.yaml index c03f6b9b..da7f9d0b 100644 --- a/.changes/unreleased/Fixes-20240227105926.yaml +++ b/.changes/unreleased/Fixes-20240227105926.yaml @@ -4,4 +4,4 @@ body: Fix incremental merge to use an `is null` check to allow for handling of n time: 2024-02-27T10:59:59.202000+00:00 custom: Author: amardatar - Issue: "7597" \ No newline at end of file + Issue: "7597" From 5063b097b9a260bf34240ad17ea7bdef5e4f720e Mon Sep 17 00:00:00 2001 From: Amar Datar <57144500+amardatar@users.noreply.github.com> Date: Thu, 12 Dec 2024 19:30:53 +1100 Subject: [PATCH 6/6] Update seed_rows param to match actual added rows --- .../incremental/test_incremental_unique_id.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/dbt-tests-adapter/dbt/tests/adapter/incremental/test_incremental_unique_id.py b/dbt-tests-adapter/dbt/tests/adapter/incremental/test_incremental_unique_id.py index 7e34a891..34807062 100644 --- a/dbt-tests-adapter/dbt/tests/adapter/incremental/test_incremental_unique_id.py +++ b/dbt-tests-adapter/dbt/tests/adapter/incremental/test_incremental_unique_id.py @@ -444,7 +444,7 @@ def fail_to_build_inc_missing_unique_key_column(self, incremental_model_name): def test__no_unique_keys(self, project): """with no unique keys, seed and model should match""" - expected_fields = self.get_expected_fields(relation="seed", seed_rows=8) + expected_fields = self.get_expected_fields(relation="seed", seed_rows=9) test_case_fields = self.get_test_fields( project, seed="seed", incremental_model="no_unique_key", update_sql_file="add_new_rows" ) @@ -454,7 +454,7 @@ def test__no_unique_keys(self, project): def test__empty_str_unique_key(self, project): """with empty string for unique key, seed and model should match""" - expected_fields = self.get_expected_fields(relation="seed", seed_rows=8) + expected_fields = self.get_expected_fields(relation="seed", seed_rows=9) test_case_fields = self.get_test_fields( project, seed="seed", @@ -467,7 +467,7 @@ def test__one_unique_key(self, project): """with one unique key, model will overwrite existing row""" expected_fields = self.get_expected_fields( - relation="one_str__overwrite", seed_rows=7, opt_model_count=1 + relation="one_str__overwrite", seed_rows=8, opt_model_count=1 ) test_case_fields = self.get_test_fields( project, @@ -492,7 +492,7 @@ def test__bad_unique_key(self, project): def test__empty_unique_key_list(self, project): """with no unique keys, seed and model should match""" - expected_fields = self.get_expected_fields(relation="seed", seed_rows=8) + expected_fields = self.get_expected_fields(relation="seed", seed_rows=9) test_case_fields = self.get_test_fields( project, seed="seed", @@ -505,7 +505,7 @@ def test__unary_unique_key_list(self, project): """with one unique key, model will overwrite existing row""" expected_fields = self.get_expected_fields( - relation="unique_key_list__inplace_overwrite", seed_rows=7, opt_model_count=1 + relation="unique_key_list__inplace_overwrite", seed_rows=8, opt_model_count=1 ) test_case_fields = self.get_test_fields( project, @@ -520,7 +520,7 @@ def test__duplicated_unary_unique_key_list(self, project): """with two of the same unique key, model will overwrite existing row""" expected_fields = self.get_expected_fields( - relation="unique_key_list__inplace_overwrite", seed_rows=7, opt_model_count=1 + relation="unique_key_list__inplace_overwrite", seed_rows=8, opt_model_count=1 ) test_case_fields = self.get_test_fields( project, @@ -535,7 +535,7 @@ def test__trinary_unique_key_list(self, project): """with three unique keys, model will overwrite existing row""" expected_fields = self.get_expected_fields( - relation="unique_key_list__inplace_overwrite", seed_rows=7, opt_model_count=1 + relation="unique_key_list__inplace_overwrite", seed_rows=8, opt_model_count=1 ) test_case_fields = self.get_test_fields( project, @@ -550,7 +550,7 @@ def test__trinary_unique_key_list_no_update(self, project): """even with three unique keys, adding distinct rows to seed does not cause seed and model to diverge""" - expected_fields = self.get_expected_fields(relation="seed", seed_rows=8) + expected_fields = self.get_expected_fields(relation="seed", seed_rows=9) test_case_fields = self.get_test_fields( project, seed="seed",