diff --git a/.changes/unreleased/Fixes-20230506-191813.yaml b/.changes/unreleased/Fixes-20230506-191813.yaml new file mode 100644 index 00000000000..c08e2ad930d --- /dev/null +++ b/.changes/unreleased/Fixes-20230506-191813.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Respect column 'quote' config in model contracts +time: 2023-05-06T19:18:13.351819+02:00 +custom: + Author: jtcohen6 + Issue: "7370" diff --git a/core/dbt/adapters/base/impl.py b/core/dbt/adapters/base/impl.py index 7bb16c7ea4e..69a17d33b61 100644 --- a/core/dbt/adapters/base/impl.py +++ b/core/dbt/adapters/base/impl.py @@ -1340,7 +1340,8 @@ def render_raw_columns_constraints(cls, raw_columns: Dict[str, Dict[str, Any]]) rendered_column_constraints = [] for v in raw_columns.values(): - rendered_column_constraint = [f"{v['name']} {v['data_type']}"] + col_name = cls.quote(cls, v["name"]) if v.get("quote") else v["name"] + rendered_column_constraint = [f"{col_name} {v['data_type']}"] for con in v.get("constraints", None): constraint = cls._parse_column_constraint(con) c = cls.process_parsed_constraint(constraint, cls.render_column_constraint) diff --git a/core/dbt/include/global_project/macros/adapters/columns.sql b/core/dbt/include/global_project/macros/adapters/columns.sql index 8605ab21d02..b98321d94c0 100644 --- a/core/dbt/include/global_project/macros/adapters/columns.sql +++ b/core/dbt/include/global_project/macros/adapters/columns.sql @@ -46,7 +46,8 @@ {%- if col['data_type'] is not defined -%} {{ col_err.append(col['name']) }} {%- endif -%} - cast(null as {{ col['data_type'] }}) as {{ col['name'] }}{{ ", " if not loop.last }} + {% set col_name = adapter.quote(col['name']) if col.get('quote') else col['name'] %} + cast(null as {{ col['data_type'] }}) as {{ col_name }}{{ ", " if not loop.last }} {%- endfor -%} {%- if (col_err | length) > 0 -%} {{ exceptions.column_type_missing(column_names=col_err) }} diff --git a/core/dbt/include/global_project/macros/materializations/models/table/create_table_as.sql b/core/dbt/include/global_project/macros/materializations/models/table/create_table_as.sql index af70f12bc31..8e15d85d9cd 100644 --- a/core/dbt/include/global_project/macros/materializations/models/table/create_table_as.sql +++ b/core/dbt/include/global_project/macros/materializations/models/table/create_table_as.sql @@ -36,15 +36,24 @@ ); {%- endmacro %} + +{% macro default__get_column_names() %} + {#- loop through user_provided_columns to get column names -#} + {%- set user_provided_columns = model['columns'] -%} + {%- for i in user_provided_columns %} + {%- set col = user_provided_columns[i] -%} + {%- set col_name = adapter.quote(col['name']) if col.get('quote') else col['name'] -%} + {{ col_name }}{{ ", " if not loop.last }} + {%- endfor -%} +{% endmacro %} + + {% macro get_select_subquery(sql) %} {{ return(adapter.dispatch('get_select_subquery', 'dbt')(sql)) }} {% endmacro %} {% macro default__get_select_subquery(sql) %} - select - {% for column in model['columns'] %} - {{ column }}{{ ", " if not loop.last }} - {% endfor %} + select {{ adapter.dispatch('get_column_names', 'dbt')() }} from ( {{ sql }} ) as model_subq diff --git a/plugins/postgres/dbt/include/postgres/macros/adapters.sql b/plugins/postgres/dbt/include/postgres/macros/adapters.sql index 07892f35b4b..6e6850a88c5 100644 --- a/plugins/postgres/dbt/include/postgres/macros/adapters.sql +++ b/plugins/postgres/dbt/include/postgres/macros/adapters.sql @@ -13,7 +13,9 @@ {% if contract_config.enforced %} {{ get_assert_columns_equivalent(sql) }} {{ get_table_columns_and_constraints() }} ; - insert into {{ relation }} {{ get_column_names() }} + insert into {{ relation }} ( + {{ adapter.dispatch('get_column_names', 'dbt')() }} + ) {%- set sql = get_select_subquery(sql) %} {% else %} as diff --git a/plugins/postgres/dbt/include/postgres/macros/utils/columns_spec_ddl.sql b/plugins/postgres/dbt/include/postgres/macros/utils/columns_spec_ddl.sql index b2f83bea4a9..e69de29bb2d 100644 --- a/plugins/postgres/dbt/include/postgres/macros/utils/columns_spec_ddl.sql +++ b/plugins/postgres/dbt/include/postgres/macros/utils/columns_spec_ddl.sql @@ -1,10 +0,0 @@ -{% macro get_column_names() %} - {# loop through user_provided_columns to get column names #} - {%- set user_provided_columns = model['columns'] -%} - ( - {% for i in user_provided_columns %} - {% set col = user_provided_columns[i] %} - {{ col['name'] }} {{ "," if not loop.last }} - {% endfor %} - ) -{% endmacro %} diff --git a/tests/adapter/dbt/tests/adapter/constraints/fixtures.py b/tests/adapter/dbt/tests/adapter/constraints/fixtures.py index 3310c59f3df..98511a7c990 100644 --- a/tests/adapter/dbt/tests/adapter/constraints/fixtures.py +++ b/tests/adapter/dbt/tests/adapter/constraints/fixtures.py @@ -179,6 +179,16 @@ '2019-01-01' as date_day """ + +# 'from' is a reserved word, so it must be quoted +my_model_with_quoted_column_name_sql = """ +select + 'blue' as "from", + 1 as id, + '2019-01-01' as date_day +""" + + model_schema_yml = """ version: 2 models: @@ -188,7 +198,6 @@ enforced: true columns: - name: id - quote: true data_type: integer description: hello constraints: @@ -278,7 +287,6 @@ name: strange_uniqueness_requirement columns: - name: id - quote: true data_type: integer description: hello constraints: @@ -303,3 +311,33 @@ - name: wrong_data_type_column_name data_type: {data_type} """ + + +model_quoted_column_schema_yml = """ +version: 2 +models: + - name: my_model + config: + contract: {enforced: true} + materialized: table + constraints: + - type: check + # this one is the on the user + expression: ("from" = 'blue') + columns: [ '"from"' ] + columns: + - name: id + data_type: integer + description: hello + constraints: + - type: not_null + tests: + - unique + - name: from # reserved word + quote: true + data_type: text + constraints: + - type: not_null + - name: date_day + data_type: text +""" diff --git a/tests/adapter/dbt/tests/adapter/constraints/test_constraints.py b/tests/adapter/dbt/tests/adapter/constraints/test_constraints.py index 79140f03bbd..add5794640b 100644 --- a/tests/adapter/dbt/tests/adapter/constraints/test_constraints.py +++ b/tests/adapter/dbt/tests/adapter/constraints/test_constraints.py @@ -23,8 +23,10 @@ my_model_incremental_wrong_name_sql, my_model_with_nulls_sql, my_model_incremental_with_nulls_sql, + my_model_with_quoted_column_name_sql, model_schema_yml, constrained_model_schema_yml, + model_quoted_column_schema_yml, ) @@ -158,7 +160,8 @@ def test__constraints_correct_column_data_types(self, project, data_types): def _normalize_whitespace(input: str) -> str: - return re.sub(r"\s+", " ", input).lower().strip() + subbed = re.sub(r"\s+", " ", input) + return re.sub(r"\s?([\(\),])\s?", r"\1", subbed).lower().strip() class BaseConstraintsRuntimeDdlEnforcement: @@ -211,15 +214,11 @@ def test__constraints_ddl(self, project, expected_sql): # the name is not what we're testing here anyways and varies based on materialization # TODO: consider refactoring this to introspect logs instead generated_sql = read_file("target", "run", "test", "models", "my_model.sql") - generated_sql_modified = _normalize_whitespace(generated_sql) - generated_sql_list = generated_sql_modified.split(" ") + generated_sql_list = generated_sql.split(" ") for idx in [n for n, x in enumerate(generated_sql_list) if "my_model" in x]: generated_sql_list[idx] = "" generated_sql_generic = " ".join(generated_sql_list) - - expected_sql_check = _normalize_whitespace(expected_sql) - - assert expected_sql_check == generated_sql_generic + assert _normalize_whitespace(expected_sql) == _normalize_whitespace(generated_sql_generic) class BaseConstraintsRollback: @@ -407,13 +406,44 @@ def test__model_constraints_ddl(self, project, expected_sql): results = run_dbt(["run", "-s", "my_model"]) assert len(results) == 1 generated_sql = read_file("target", "run", "test", "models", "my_model.sql") - generated_sql_list = _normalize_whitespace(generated_sql).split(" ") - generated_sql_list = [ - "" if "my_model" in s else s for s in generated_sql_list - ] + generated_sql_list = generated_sql.split(" ") + for idx in [n for n, x in enumerate(generated_sql_list) if "my_model" in x]: + generated_sql_list[idx] = "" generated_sql_generic = " ".join(generated_sql_list) - assert _normalize_whitespace(expected_sql) == generated_sql_generic + assert _normalize_whitespace(expected_sql) == _normalize_whitespace(generated_sql_generic) class TestModelConstraintsRuntimeEnforcement(BaseModelConstraintsRuntimeEnforcement): pass + + +class TestConstraintQuotedColumn(BaseConstraintsRuntimeDdlEnforcement): + @pytest.fixture(scope="class") + def models(self): + return { + "my_model.sql": my_model_with_quoted_column_name_sql, + "constraints_schema.yml": model_quoted_column_schema_yml, + } + + @pytest.fixture(scope="class") + def expected_sql(self): + return """ +create table ( + id integer not null, + "from" text not null, + date_day text, + check ("from" = 'blue') +) ; +insert into ( + id, "from", date_day +) +( + select id, "from", date_day + from ( + select + 'blue' as "from", + 1 as id, + '2019-01-01' as date_day + ) as model_subq +); +"""