From 7502c8ba781e857633e6d57e582274cc17f13524 Mon Sep 17 00:00:00 2001 From: Sam Debruyn Date: Mon, 15 May 2023 13:51:49 +0200 Subject: [PATCH 1/8] add tests for new incremental features --- tests/functional/adapter/test_incremental.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/functional/adapter/test_incremental.py b/tests/functional/adapter/test_incremental.py index 873a6cfd..06cc52d1 100644 --- a/tests/functional/adapter/test_incremental.py +++ b/tests/functional/adapter/test_incremental.py @@ -1,5 +1,24 @@ +import pytest +from dbt.tests.adapter.incremental.test_incremental_on_schema_change import ( + BaseIncrementalOnSchemaChange, +) +from dbt.tests.adapter.incremental.test_incremental_predicates import BaseIncrementalPredicates from dbt.tests.adapter.incremental.test_incremental_unique_id import BaseIncrementalUniqueKey class TestBaseIncrementalUniqueKeySQLServer(BaseIncrementalUniqueKey): pass + + +class TestIncrementalOnSchemaChangeSQLServer(BaseIncrementalOnSchemaChange): + pass + + +class TestIncrementalPredicatesDeleteInsertSQLServer(BaseIncrementalPredicates): + pass + + +class TestPredicatesDeleteInsertSQLServer(BaseIncrementalPredicates): + @pytest.fixture(scope="class") + def project_config_update(self): + return {"models": {"+predicates": ["id != 2"], "+incremental_strategy": "delete+insert"}} From 4b4458b327c932c354c322bdec9b5a91f8a40e44 Mon Sep 17 00:00:00 2001 From: Sam Debruyn Date: Mon, 15 May 2023 18:50:36 +0200 Subject: [PATCH 2/8] base incremental predicate impl --- .../models/incremental/merge.sql | 78 ++++++++++--------- 1 file changed, 41 insertions(+), 37 deletions(-) diff --git a/dbt/include/sqlserver/macros/materializations/models/incremental/merge.sql b/dbt/include/sqlserver/macros/materializations/models/incremental/merge.sql index 2f0567c7..8ef884a2 100644 --- a/dbt/include/sqlserver/macros/materializations/models/incremental/merge.sql +++ b/dbt/include/sqlserver/macros/materializations/models/incremental/merge.sql @@ -5,49 +5,53 @@ https://getdbt.slack.com/archives/C50NEBJGG/p1636045535056600 #} -{% macro sqlserver__get_merge_sql(target, source, unique_key, dest_columns, predicates) %} - {{ default__get_merge_sql(target, source, unique_key, dest_columns, predicates) }}; +{% macro sqlserver__get_merge_sql(target, source, unique_key, dest_columns, incremental_predicates) %} + {{ default__get_merge_sql(target, source, unique_key, dest_columns, incremental_predicates) }}; {% endmacro %} -{% macro sqlserver__get_delete_insert_merge_sql(target, source, unique_key, dest_columns, incremental_predicates) %} - {% if incremental_predicates %} - {{ exceptions.raise_not_implemented('incremental_predicates are not implemented in dbt-sqlserver') }} - {% endif %} - - {%- set dest_cols_csv = get_quoted_csv(dest_columns | map(attribute="name")) -%} +{% macro sqlserver__get_insert_overwrite_merge_sql(target, source, dest_columns, predicates, include_sql_header) %} + {{ default__get_insert_overwrite_merge_sql(target, source, dest_columns, predicates, include_sql_header) }}; +{% endmacro %} - {% if unique_key %} - {% if unique_key is sequence and unique_key is not string %} - delete from {{ target }} - where exists ( - SELECT NULL - FROM - {{ source }} - WHERE +{% macro sqlserver__get_delete_insert_merge_sql(target, source, unique_key, dest_columns, incremental_predicates) %} + {%- set dest_cols_csv = get_quoted_csv(dest_columns | map(attribute="name")) -%} + + {% if unique_key %} + {% if unique_key is sequence and unique_key is not string %} + delete from {{ target }} + where exists ( + select null + from {{ source }} + where {% for key in unique_key %} {{ source }}.{{ key }} = {{ target }}.{{ key }} {{ "and " if not loop.last }} {% endfor %} - ); - {% else %} - delete from {{ target }} - where ( - {{ unique_key }}) in ( - select ({{ unique_key }}) - from {{ source }} - ); - - {% endif %} - {% endif %} - insert into {{ target }} ({{ dest_cols_csv }}) - ( - select {{ dest_cols_csv }} - from {{ source }} - ) - -{% endmacro %} - -{% macro sqlserver__get_insert_overwrite_merge_sql(target, source, dest_columns, predicates, include_sql_header) %} - {{ default__get_insert_overwrite_merge_sql(target, source, dest_columns, predicates, include_sql_header) }}; + ) + {% if incremental_predicates %} + {% for predicate in incremental_predicates %} + and {{ predicate }} + {% endfor %} + {% endif %}; + {% else %} + delete from {{ target }} + where ( + {{ unique_key }}) in ( + select ({{ unique_key }}) + from {{ source }} + ) + {%- if incremental_predicates %} + {% for predicate in incremental_predicates %} + and {{ predicate }} + {% endfor %} + {%- endif -%}; + {% endif %} + {% endif %} + + insert into {{ target }} ({{ dest_cols_csv }}) + ( + select {{ dest_cols_csv }} + from {{ source }} + ) {% endmacro %} From d29fdb75dbe20d8d623a315adbdd8addc89d2dbc Mon Sep 17 00:00:00 2001 From: Sam Debruyn Date: Mon, 15 May 2023 19:08:15 +0200 Subject: [PATCH 3/8] fix a test --- tests/functional/adapter/test_incremental.py | 59 +++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/tests/functional/adapter/test_incremental.py b/tests/functional/adapter/test_incremental.py index 06cc52d1..a020795f 100644 --- a/tests/functional/adapter/test_incremental.py +++ b/tests/functional/adapter/test_incremental.py @@ -1,17 +1,74 @@ import pytest +from dbt.tests.adapter.incremental.fixtures import ( + _MODELS__A, + _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS, + _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS_REMOVE_ONE, + _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS_REMOVE_ONE_TARGET, + _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS_TARGET, + _MODELS__INCREMENTAL_FAIL, + _MODELS__INCREMENTAL_IGNORE_TARGET, + _MODELS__INCREMENTAL_SYNC_ALL_COLUMNS, + _MODELS__INCREMENTAL_SYNC_ALL_COLUMNS_TARGET, + _MODELS__INCREMENTAL_SYNC_REMOVE_ONLY, + _MODELS__INCREMENTAL_SYNC_REMOVE_ONLY_TARGET, +) from dbt.tests.adapter.incremental.test_incremental_on_schema_change import ( BaseIncrementalOnSchemaChange, ) from dbt.tests.adapter.incremental.test_incremental_predicates import BaseIncrementalPredicates from dbt.tests.adapter.incremental.test_incremental_unique_id import BaseIncrementalUniqueKey +_MODELS__INCREMENTAL_IGNORE = """ +{{ + config( + materialized='incremental', + unique_key='id', + on_schema_change='ignore' + ) +}} + +WITH source_data AS (SELECT * FROM {{ ref('model_a') }} ) + +{% if is_incremental() %} + +SELECT + id, + field1, + field2, + field3, + field4 +FROM source_data +WHERE id NOT IN (SELECT id from {{ this }} ) + +{% else %} + +SELECT TOP 3 id, field1, field2 FROM source_data + +{% endif %} +""" + class TestBaseIncrementalUniqueKeySQLServer(BaseIncrementalUniqueKey): pass class TestIncrementalOnSchemaChangeSQLServer(BaseIncrementalOnSchemaChange): - pass + @pytest.fixture(scope="class") + def models(self): + return { + "incremental_sync_remove_only.sql": _MODELS__INCREMENTAL_SYNC_REMOVE_ONLY, + "incremental_ignore.sql": _MODELS__INCREMENTAL_IGNORE, + "incremental_sync_remove_only_target.sql": _MODELS__INCREMENTAL_SYNC_REMOVE_ONLY_TARGET, # noqa: E501 + "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_append_new_columns_remove_one.sql": _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS_REMOVE_ONE, # noqa: E501 + "model_a.sql": _MODELS__A, + "incremental_append_new_columns_target.sql": _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS_TARGET, # noqa: E501 + "incremental_append_new_columns.sql": _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS, + "incremental_sync_all_columns_target.sql": _MODELS__INCREMENTAL_SYNC_ALL_COLUMNS_TARGET, # noqa: E501 + "incremental_append_new_columns_remove_one_target.sql": _MODELS__INCREMENTAL_APPEND_NEW_COLUMNS_REMOVE_ONE_TARGET, # noqa: E501 + } class TestIncrementalPredicatesDeleteInsertSQLServer(BaseIncrementalPredicates): From c103963e89d985e5977580fe24c66787e742a585 Mon Sep 17 00:00:00 2001 From: Sam Debruyn Date: Mon, 15 May 2023 20:44:56 +0200 Subject: [PATCH 4/8] fix more tests --- dbt/adapters/sqlserver/sql_server_column.py | 9 ++++++++- dbt/include/sqlserver/macros/adapters/columns.sql | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/dbt/adapters/sqlserver/sql_server_column.py b/dbt/adapters/sqlserver/sql_server_column.py index 93a3924f..4f2e01a4 100644 --- a/dbt/adapters/sqlserver/sql_server_column.py +++ b/dbt/adapters/sqlserver/sql_server_column.py @@ -1,4 +1,4 @@ -from typing import ClassVar, Dict +from typing import Any, ClassVar, Dict from dbt.adapters.base import Column @@ -11,3 +11,10 @@ class SQLServerColumn(Column): "INTEGER": "INT", "BOOLEAN": "BIT", } + + @classmethod + def string_type(cls, size: int) -> str: + return f"varchar({size if size > 0 else 'MAX'})" + + def literal(self, value: Any) -> str: + return "cast('{}' as {})".format(value, self.data_type) diff --git a/dbt/include/sqlserver/macros/adapters/columns.sql b/dbt/include/sqlserver/macros/adapters/columns.sql index d8c58d40..1e253881 100644 --- a/dbt/include/sqlserver/macros/adapters/columns.sql +++ b/dbt/include/sqlserver/macros/adapters/columns.sql @@ -55,3 +55,18 @@ {%- endcall -%} {% endmacro %} + + +{% macro sqlserver__alter_relation_add_remove_columns(relation, add_columns, remove_columns) %} + {% call statement('add_drop_columns') -%} + {% if add_columns %} + alter {{ relation.type }} {{ relation }} + add {% for column in add_columns %}{{ column.name }} {{ column.data_type }}{{ ', ' if not loop.last }}{% endfor %}; + {% endif %} + + {% if remove_columns %} + alter {{ relation.type }} {{ relation }} + drop column {% for column in remove_columns %}{{ column.name }}{{ ',' if not loop.last }}{% endfor %}; + {% endif %} + {%- endcall -%} +{% endmacro %} From d291c23dde161c6527c4a5289cf8b1c9d65f377d Mon Sep 17 00:00:00 2001 From: Sam Debruyn Date: Mon, 15 May 2023 20:54:35 +0200 Subject: [PATCH 5/8] fix another test --- tests/functional/adapter/test_incremental.py | 43 +++++++++++++++++++- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/tests/functional/adapter/test_incremental.py b/tests/functional/adapter/test_incremental.py index a020795f..c4d14c2d 100644 --- a/tests/functional/adapter/test_incremental.py +++ b/tests/functional/adapter/test_incremental.py @@ -8,9 +8,7 @@ _MODELS__INCREMENTAL_FAIL, _MODELS__INCREMENTAL_IGNORE_TARGET, _MODELS__INCREMENTAL_SYNC_ALL_COLUMNS, - _MODELS__INCREMENTAL_SYNC_ALL_COLUMNS_TARGET, _MODELS__INCREMENTAL_SYNC_REMOVE_ONLY, - _MODELS__INCREMENTAL_SYNC_REMOVE_ONLY_TARGET, ) from dbt.tests.adapter.incremental.test_incremental_on_schema_change import ( BaseIncrementalOnSchemaChange, @@ -47,6 +45,47 @@ {% endif %} """ +_MODELS__INCREMENTAL_SYNC_REMOVE_ONLY_TARGET = """ +{{ + config(materialized='table') +}} + +with source_data as ( + + select * from {{ ref('model_a') }} + +) + +{% set string_type = dbt.type_string() %} + +select id + ,cast(field1 as {{string_type}}) as field1 + +from source_data +""" + +_MODELS__INCREMENTAL_SYNC_ALL_COLUMNS_TARGET = """ +{{ + config(materialized='table') +}} + +with source_data as ( + + select * from {{ ref('model_a') }} + +) + +{% set string_type = dbt.type_string() %} + +select id + ,cast(field1 as {{string_type}}) as field1 + --,field2 + ,cast(case when id <= 3 then null else field3 end as {{string_type}}) as field3 + ,cast(case when id <= 3 then null else field4 end as {{string_type}}) as field4 + +from source_data +""" + class TestBaseIncrementalUniqueKeySQLServer(BaseIncrementalUniqueKey): pass From a72fd629f960b615ab7f673a950bdb93054b0661 Mon Sep 17 00:00:00 2001 From: Sam Debruyn Date: Mon, 15 May 2023 21:10:24 +0200 Subject: [PATCH 6/8] fix remaining tests --- .../sqlserver/macros/adapters/columns.sql | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/dbt/include/sqlserver/macros/adapters/columns.sql b/dbt/include/sqlserver/macros/adapters/columns.sql index 1e253881..2e586f95 100644 --- a/dbt/include/sqlserver/macros/adapters/columns.sql +++ b/dbt/include/sqlserver/macros/adapters/columns.sql @@ -46,12 +46,16 @@ {%- set tmp_column = column_name + "__dbt_alter" -%} {% call statement('alter_column_type') -%} - - alter {{ relation.type }} {{ relation }} add {{ tmp_column }} {{ new_column_type }}; - update {{ relation }} set {{ tmp_column }} = {{ column_name }}; - alter {{ relation.type }} {{ relation }} drop column {{ column_name }}; + alter {{ relation.type }} {{ relation }} add "{{ tmp_column }}" {{ new_column_type }}; + {%- endcall -%} + {% call statement('alter_column_type') -%} + update {{ relation }} set "{{ tmp_column }}" = "{{ column_name }}"; + {%- endcall -%} + {% call statement('alter_column_type') -%} + alter {{ relation.type }} {{ relation }} drop column "{{ column_name }}"; + {%- endcall -%} + {% call statement('alter_column_type') -%} exec sp_rename '{{ relation | replace('"', '') }}.{{ tmp_column }}', '{{ column_name }}', 'column' - {%- endcall -%} {% endmacro %} @@ -61,12 +65,12 @@ {% call statement('add_drop_columns') -%} {% if add_columns %} alter {{ relation.type }} {{ relation }} - add {% for column in add_columns %}{{ column.name }} {{ column.data_type }}{{ ', ' if not loop.last }}{% endfor %}; + add {% for column in add_columns %}"{{ column.name }}" {{ column.data_type }}{{ ', ' if not loop.last }}{% endfor %}; {% endif %} {% if remove_columns %} alter {{ relation.type }} {{ relation }} - drop column {% for column in remove_columns %}{{ column.name }}{{ ',' if not loop.last }}{% endfor %}; + drop column {% for column in remove_columns %}"{{ column.name }}"{{ ',' if not loop.last }}{% endfor %}; {% endif %} {%- endcall -%} {% endmacro %} From f1b9b75a0f4a2f6aa6822218913dad1b4796cf0c Mon Sep 17 00:00:00 2001 From: Sam Debruyn Date: Mon, 15 May 2023 21:19:30 +0200 Subject: [PATCH 7/8] add default arg --- .../macros/materializations/models/incremental/merge.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbt/include/sqlserver/macros/materializations/models/incremental/merge.sql b/dbt/include/sqlserver/macros/materializations/models/incremental/merge.sql index 8ef884a2..c19ff81c 100644 --- a/dbt/include/sqlserver/macros/materializations/models/incremental/merge.sql +++ b/dbt/include/sqlserver/macros/materializations/models/incremental/merge.sql @@ -5,7 +5,7 @@ https://getdbt.slack.com/archives/C50NEBJGG/p1636045535056600 #} -{% macro sqlserver__get_merge_sql(target, source, unique_key, dest_columns, incremental_predicates) %} +{% macro sqlserver__get_merge_sql(target, source, unique_key, dest_columns, incremental_predicates=none) %} {{ default__get_merge_sql(target, source, unique_key, dest_columns, incremental_predicates) }}; {% endmacro %} From c28fe8de6d1f36e8ac3fbf2804a8ac9780b50a90 Mon Sep 17 00:00:00 2001 From: Sam Debruyn Date: Mon, 15 May 2023 21:44:10 +0200 Subject: [PATCH 8/8] fix seed tests --- .../macros/materializations/models/incremental/merge.sql | 2 +- tests/functional/adapter/test_seed.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dbt/include/sqlserver/macros/materializations/models/incremental/merge.sql b/dbt/include/sqlserver/macros/materializations/models/incremental/merge.sql index c19ff81c..38202a9f 100644 --- a/dbt/include/sqlserver/macros/materializations/models/incremental/merge.sql +++ b/dbt/include/sqlserver/macros/materializations/models/incremental/merge.sql @@ -13,7 +13,7 @@ {{ default__get_insert_overwrite_merge_sql(target, source, dest_columns, predicates, include_sql_header) }}; {% endmacro %} -{% macro sqlserver__get_delete_insert_merge_sql(target, source, unique_key, dest_columns, incremental_predicates) %} +{% macro sqlserver__get_delete_insert_merge_sql(target, source, unique_key, dest_columns, incremental_predicates=none) %} {%- set dest_cols_csv = get_quoted_csv(dest_columns | map(attribute="name")) -%} {% if unique_key %} diff --git a/tests/functional/adapter/test_seed.py b/tests/functional/adapter/test_seed.py index 0b08e91a..8157566c 100644 --- a/tests/functional/adapter/test_seed.py +++ b/tests/functional/adapter/test_seed.py @@ -49,7 +49,7 @@ {% endfor %} {% set col_type = col_types.get(column_name) %} - {% set col_type = 'text' if col_type and 'character varying' in col_type else col_type %} + {% set col_type = 'text' if col_type and 'varchar' in col_type else col_type %} {% set validation_message = 'Got a column type of ' ~ col_type ~ ', expected ' ~ type %}