Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add quoting for sync all columns #8914

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20231026-003750.yaml
Original file line number Diff line number Diff line change
@@ -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"
4 changes: 2 additions & 2 deletions core/dbt/include/global_project/macros/adapters/columns.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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 -%}
Expand Down
77 changes: 77 additions & 0 deletions tests/adapter/dbt/tests/adapter/incremental/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)

Expand All @@ -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,
}

Expand Down Expand Up @@ -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"
Expand All @@ -91,6 +103,7 @@ def test_run_incremental_append_new_columns(self, project):

def test_run_incremental_sync_all_columns(self, 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):
Expand Down
Loading