From d0e7d024544a1c959d94cc9ff826449dca4b9793 Mon Sep 17 00:00:00 2001 From: DTchebotarev Date: Wed, 25 Oct 2023 23:44:48 -0400 Subject: [PATCH 1/3] Add quoting for sync all columns --- .../macros/adapters/columns.sql | 4 +- .../dbt/tests/adapter/incremental/fixtures.py | 77 +++++++++++++++++++ .../test_incremental_on_schema_change.py | 17 +++- 3 files changed, 94 insertions(+), 4 deletions(-) diff --git a/core/dbt/include/global_project/macros/adapters/columns.sql b/core/dbt/include/global_project/macros/adapters/columns.sql index e1099649cf0..7f473409c72 100644 --- a/core/dbt/include/global_project/macros/adapters/columns.sql +++ b/core/dbt/include/global_project/macros/adapters/columns.sql @@ -123,11 +123,11 @@ alter {{ relation.type }} {{ relation }} {% for column in add_columns %} - add column {{ column.name }} {{ column.data_type }}{{ ',' if not loop.last }} + add column {{ adapter.quote(column.name) }} {{ column.data_type }}{{ ',' if not loop.last }} {% endfor %}{{ ',' if add_columns and remove_columns }} {% for column in remove_columns %} - drop column {{ column.name }}{{ ',' if not loop.last }} + drop column {{ adapter.quote(column.name) }}{{ ',' if not loop.last }} {% endfor %} {%- endset -%} diff --git a/tests/adapter/dbt/tests/adapter/incremental/fixtures.py b/tests/adapter/dbt/tests/adapter/incremental/fixtures.py index 6e130266df2..bf872c59e33 100644 --- a/tests/adapter/dbt/tests/adapter/incremental/fixtures.py +++ b/tests/adapter/dbt/tests/adapter/incremental/fixtures.py @@ -149,6 +149,38 @@ {% endif %} """ +_MODELS__INCREMENTAL_SYNC_ALL_QUOTED_COLUMNS = """ +{{ + config( + materialized='incremental', + unique_key='id', + on_schema_change='sync_all_columns' + + ) +}} + +WITH source_data AS (SELECT * FROM {{ ref('model_b') }} ) + +{% set string_type = dbt.type_string() %} + +{% if is_incremental() %} + +SELECT id + ,CAST ('new incremental field' AS {{string_type}}) AS "new incremental quoted field" + +FROM source_data WHERE id NOT IN (SELECT id FROM {{ this }} ) +ORDER BY id + +{% else %} + +SELECT * + +FROM source_data WHERE id <= 3 +ORDER BY id + +{% endif %} +""" + _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS_REMOVE_ONE = """ {{ config( @@ -205,6 +237,31 @@ from source_data """ +_MODELS__B = """ +{{ + config(materialized='table') +}} + +with source_data as ( + + select 1 as id, 'aaa' as "space field", 'bbb' as "special!@#.%^*field", 111 as "3field", 'TTT' as "фиелд4" + union all select 2 as id, 'ccc' as "space field", 'ddd' as "special!@#.%^*field", 222 as "3field", 'UUU' as "фиелд4" + union all select 3 as id, 'eee' as "space field", 'fff' as "special!@#.%^*field", 333 as "3field", 'VVV' as "фиелд4" + union all select 4 as id, 'ggg' as "space field", 'hhh' as "special!@#.%^*field", 444 as "3field", 'WWW' as "фиелд4" + union all select 5 as id, 'iii' as "space field", 'jjj' as "special!@#.%^*field", 555 as "3field", 'XXX' as "фиелд4" + union all select 6 as id, 'kkk' as "space field", 'lll' as "special!@#.%^*field", 666 as "3field", 'YYY' as "фиелд4" + +) + +select id + ,"space field" + ,"special!@#.%^*field" + ,"3field" + ,"фиелд4" + +from source_data +""" + _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS_TARGET = """ {{ config(materialized='table') @@ -282,6 +339,26 @@ order by id """ +_MODELS__INCREMENTAL_SYNC_ALL_QUOTED_COLUMNS_TARGET = """ +{{ + config(materialized='table') +}} + +with source_data as ( + + select * from {{ ref('model_b') }} + +) + +{% set string_type = dbt.type_string() %} + +select id + ,cast(case when id <= 3 then null else 'new incremental field' end as {{string_type}}) as "new incremental quoted field" + +from source_data +order by id +""" + _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS_REMOVE_ONE_TARGET = """ {{ config(materialized='table') diff --git a/tests/adapter/dbt/tests/adapter/incremental/test_incremental_on_schema_change.py b/tests/adapter/dbt/tests/adapter/incremental/test_incremental_on_schema_change.py index 4fbefbe7651..94e00ef035b 100644 --- a/tests/adapter/dbt/tests/adapter/incremental/test_incremental_on_schema_change.py +++ b/tests/adapter/dbt/tests/adapter/incremental/test_incremental_on_schema_change.py @@ -12,11 +12,14 @@ _MODELS__INCREMENTAL_IGNORE_TARGET, _MODELS__INCREMENTAL_FAIL, _MODELS__INCREMENTAL_SYNC_ALL_COLUMNS, + _MODELS__INCREMENTAL_SYNC_ALL_QUOTED_COLUMNS, _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS_REMOVE_ONE, _MODELS__A, + _MODELS__B, _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS_TARGET, _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS, _MODELS__INCREMENTAL_SYNC_ALL_COLUMNS_TARGET, + _MODELS__INCREMENTAL_SYNC_ALL_QUOTED_COLUMNS_TARGET, _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS_REMOVE_ONE_TARGET, ) @@ -31,11 +34,14 @@ def models(self): "incremental_ignore_target.sql": _MODELS__INCREMENTAL_IGNORE_TARGET, "incremental_fail.sql": _MODELS__INCREMENTAL_FAIL, "incremental_sync_all_columns.sql": _MODELS__INCREMENTAL_SYNC_ALL_COLUMNS, + "incremental_sync_all_quoted_columns.sql": _MODELS__INCREMENTAL_SYNC_ALL_QUOTED_COLUMNS, "incremental_append_new_columns_remove_one.sql": _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS_REMOVE_ONE, "model_a.sql": _MODELS__A, + "model_b.sql": _MODELS__B, "incremental_append_new_columns_target.sql": _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS_TARGET, "incremental_append_new_columns.sql": _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS, "incremental_sync_all_columns_target.sql": _MODELS__INCREMENTAL_SYNC_ALL_COLUMNS_TARGET, + "incremental_sync_all_quoted_columns_target.sql": _MODELS__INCREMENTAL_SYNC_ALL_QUOTED_COLUMNS_TARGET, "incremental_append_new_columns_remove_one_target.sql": _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS_REMOVE_ONE_TARGET, } @@ -71,6 +77,12 @@ def run_incremental_sync_all_columns(self, project): compare_target = "incremental_sync_all_columns_target" self.run_twice_and_assert(select, compare_source, compare_target, project) + def run_incremental_sync_all_quoted_columns(self, project): + select = "model_b incremental_sync_all_quoted_columns incremental_sync_all_quoted_columns_target" + compare_source = "incremental_sync_all_quoted_columns" + compare_target = "incremental_sync_all_quoted_columns_target" + self.run_twice_and_assert(select, compare_source, compare_target, project) + def run_incremental_sync_remove_only(self, project): select = "model_a incremental_sync_remove_only incremental_sync_remove_only_target" compare_source = "incremental_sync_remove_only" @@ -90,8 +102,9 @@ def test_run_incremental_append_new_columns(self, project): self.run_incremental_append_new_columns_remove_one(project) def test_run_incremental_sync_all_columns(self, project): - self.run_incremental_sync_all_columns(project) - self.run_incremental_sync_remove_only(project) + # self.run_incremental_sync_all_columns(project) + self.run_incremental_sync_all_quoted_columns(project) + # self.run_incremental_sync_remove_only(project) def test_run_incremental_fail_on_schema_change(self, project): select = "model_a incremental_fail" From 3c81e88d97006545ef76e01796949dc42db23d76 Mon Sep 17 00:00:00 2001 From: Dmitri Tchebotarev Date: Thu, 26 Oct 2023 00:38:10 -0400 Subject: [PATCH 2/3] Add changie changelog changes --- .changes/unreleased/Fixes-20231026-003750.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Fixes-20231026-003750.yaml diff --git a/.changes/unreleased/Fixes-20231026-003750.yaml b/.changes/unreleased/Fixes-20231026-003750.yaml new file mode 100644 index 00000000000..739eee253e4 --- /dev/null +++ b/.changes/unreleased/Fixes-20231026-003750.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Fix bug where incremental column changes did not quote columns. +time: 2023-10-26T00:37:50.705609-04:00 +custom: + Author: DTchebotarev + Issue: "4423" From 0f4b601eff992f9b074d72cdf4d2c81c892cc751 Mon Sep 17 00:00:00 2001 From: Dmitri Tchebotarev Date: Thu, 26 Oct 2023 00:40:10 -0400 Subject: [PATCH 3/3] Uncomment some tests. Oops --- .../adapter/incremental/test_incremental_on_schema_change.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/adapter/dbt/tests/adapter/incremental/test_incremental_on_schema_change.py b/tests/adapter/dbt/tests/adapter/incremental/test_incremental_on_schema_change.py index 94e00ef035b..8e744a83dee 100644 --- a/tests/adapter/dbt/tests/adapter/incremental/test_incremental_on_schema_change.py +++ b/tests/adapter/dbt/tests/adapter/incremental/test_incremental_on_schema_change.py @@ -102,9 +102,9 @@ def test_run_incremental_append_new_columns(self, project): self.run_incremental_append_new_columns_remove_one(project) def test_run_incremental_sync_all_columns(self, project): - # self.run_incremental_sync_all_columns(project) + self.run_incremental_sync_all_columns(project) self.run_incremental_sync_all_quoted_columns(project) - # self.run_incremental_sync_remove_only(project) + self.run_incremental_sync_remove_only(project) def test_run_incremental_fail_on_schema_change(self, project): select = "model_a incremental_fail"