From 90cafd3922d3733a972d4223cf3201e2015265a8 Mon Sep 17 00:00:00 2001 From: Matthew Lilius Date: Wed, 27 Aug 2025 22:28:49 +0000 Subject: [PATCH 1/4] Fix column quoting in incremental merge strategy for SQL keywords - Quote column names in incremental merge strategy to handle SQL keywords - Add comprehensive tests for incremental materialization with keyword columns - Test validates proper handling of columns named 'language' and 'count' - Ensures incremental updates work correctly with quoted column names --- .../incremental/strategies.sql | 2 +- .../materialization/test_incremental.py | 56 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/dbt/include/dremio/macros/materializations/incremental/strategies.sql b/dbt/include/dremio/macros/materializations/incremental/strategies.sql index 483ce95b..32e757e2 100644 --- a/dbt/include/dremio/macros/materializations/incremental/strategies.sql +++ b/dbt/include/dremio/macros/materializations/incremental/strategies.sql @@ -64,7 +64,7 @@ limitations under the License.*/ ({{ dest_cols_csv }}) values ({% for column_name in dest_columns | map(attribute="name") -%} - DBT_INTERNAL_SOURCE.{{ column_name }} + DBT_INTERNAL_SOURCE.{{ adapter.quote(column_name) }} {%- if not loop.last %}, {%- endif %} {%- endfor %}) diff --git a/tests/functional/adapter/materialization/test_incremental.py b/tests/functional/adapter/materialization/test_incremental.py index 52707ab1..ab17d68b 100644 --- a/tests/functional/adapter/materialization/test_incremental.py +++ b/tests/functional/adapter/materialization/test_incremental.py @@ -64,6 +64,28 @@ {% endif %} """ +models__incremental_keyword_columns_sql = """ +{{ config( + materialized = 'incremental', + unique_key = 'id', + incremental_strategy='merge' +) }} + +{% if not is_incremental() %} + +-- data for first invocation of model + +select 1 as id, 'test' as "language", 95 as "count" + +{% else %} + +-- data for subsequent incremental update + +select 2 as id, 'test2' as "language", 88 as "count" + +{% endif %} +""" + ResultHolder = namedtuple( "ResultHolder", [ @@ -204,3 +226,37 @@ def check_scenario_correctness(self, expected_fields, test_case_fields, project) check_relations_equal( project.adapter, [expected_fields.relation, test_case_fields.relation] ) + + +class TestIncrementalColumnQuoting: + @pytest.fixture(scope="class") + def models(self): + return { + "incremental_keyword_columns.sql": models__incremental_keyword_columns_sql, + "schema.yml": schema_base_yml + } + + def test_incremental_keyword_columns(self, project): + results = run_dbt(["run", "--select", "incremental_keyword_columns"]) + assert len(results) == 1 + + results = run_dbt(["run", "--select", "incremental_keyword_columns"]) + assert len(results) == 1 + + relation = relation_from_name(project.adapter, "incremental_keyword_columns") + result = project.run_sql( + f"select count(*) as num_rows from {relation}", fetch="one" + ) + assert result[0] == 2 + + rows = project.run_sql( + f'select id, "language", "count" from {relation} order by id', + fetch="all" + ) + assert len(rows) == 2 + assert rows[0][0] == 1 + assert rows[0][1] == 'test' + assert rows[0][2] == 95 + assert rows[1][0] == 2 + assert rows[1][1] == 'test2' + assert rows[1][2] == 88 From 938dee22dfc85c01b65d560c49eb98e99255a544 Mon Sep 17 00:00:00 2001 From: Matthew Lilius Date: Wed, 27 Aug 2025 21:52:02 -0400 Subject: [PATCH 2/4] Update CHANGELOG.md for column quoting fix --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c029ff1..b2198269 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Changes +- Fix column quoting in incremental merge strategy for SQL keywords - ensures proper handling of columns with SQL keyword names in incremental materializations - [#299](https://github.com/dremio/dbt-dremio/pull/299) Enhance persist_docs macro to wrap model and column metadata (including descriptions, tags and tests) into a Markdown wiki for Dremio. - Refactored CI - Fixed tests for hooks and grants From 5173e39eeef4a254cd1f7242aca4f17c7e0d6787 Mon Sep 17 00:00:00 2001 From: Matthew Lilius Date: Mon, 24 Nov 2025 20:39:10 -0500 Subject: [PATCH 3/4] Fix quoting for unique keys and column names in incremental merge strategy - Updated the incremental merge strategy to quote unique keys and column names using the adapter's quote function, ensuring compatibility with SQL keywords. - Adjusted the macro signature to include an optional parameter for incremental predicates, enhancing flexibility in SQL generation. --- .../macros/materializations/incremental/strategies.sql | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dbt/include/dremio/macros/materializations/incremental/strategies.sql b/dbt/include/dremio/macros/materializations/incremental/strategies.sql index 32e757e2..d3fb1ca9 100644 --- a/dbt/include/dremio/macros/materializations/incremental/strategies.sql +++ b/dbt/include/dremio/macros/materializations/incremental/strategies.sql @@ -32,13 +32,13 @@ limitations under the License.*/ {% 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.{{ adapter.quote(key) }} = DBT_INTERNAL_DEST.{{ adapter.quote(key) }} {% 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.{{ adapter.quote(unique_key) }} = DBT_INTERNAL_DEST.{{ adapter.quote(unique_key) }} {% endset %} {% do predicates.append(unique_key_match) %} {% endif %} @@ -55,7 +55,7 @@ limitations under the License.*/ {% if unique_key %} when matched then update set {% for column_name in update_columns -%} - {{ column_name }} = DBT_INTERNAL_SOURCE.{{ column_name }} + {{ adapter.quote(column_name) }} = DBT_INTERNAL_SOURCE.{{ adapter.quote(column_name) }} {%- if not loop.last %}, {%- endif %} {%- endfor %} {% endif %} @@ -70,11 +70,11 @@ limitations under the License.*/ {% endmacro %} -{% macro dbt_dremio_get_incremental_sql(strategy, source, target, dest_columns, unique_key) %} +{% macro dbt_dremio_get_incremental_sql(strategy, source, target, dest_columns, unique_key, incremental_predicates=none) %} {%- if strategy == 'append' -%} {{ dremio__get_incremental_append_sql(source, target, dest_columns) }} {%- elif strategy == 'merge' -%} - {{dremio__get_incremental_merge_sql(target, source, unique_key, dest_columns, incremental_predicates=none)}} + {{dremio__get_incremental_merge_sql(target, source, unique_key, dest_columns, incremental_predicates)}} {%- else -%} {% set no_sql_for_strategy_msg -%} No known SQL for the incremental strategy provided: {{ strategy }} From d10a88032154317aa5f563d6ef9857b06b934ece Mon Sep 17 00:00:00 2001 From: Matthew Lilius Date: Mon, 24 Nov 2025 21:32:38 -0500 Subject: [PATCH 4/4] Fix column assignment in incremental merge strategy by removing unnecessary quoting for column names. This change ensures that the column names are directly referenced, improving clarity and consistency in the SQL generation process. --- .../dremio/macros/materializations/incremental/strategies.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbt/include/dremio/macros/materializations/incremental/strategies.sql b/dbt/include/dremio/macros/materializations/incremental/strategies.sql index d3fb1ca9..d4ec6c03 100644 --- a/dbt/include/dremio/macros/materializations/incremental/strategies.sql +++ b/dbt/include/dremio/macros/materializations/incremental/strategies.sql @@ -55,7 +55,7 @@ limitations under the License.*/ {% if unique_key %} when matched then update set {% for column_name in update_columns -%} - {{ adapter.quote(column_name) }} = DBT_INTERNAL_SOURCE.{{ adapter.quote(column_name) }} + {{ column_name }} = DBT_INTERNAL_SOURCE.{{ column_name }} {%- if not loop.last %}, {%- endif %} {%- endfor %} {% endif %}