Skip to content

Commit

Permalink
Respect column 'quote' config in model contracts
Browse files Browse the repository at this point in the history
  • Loading branch information
jtcohen6 committed May 10, 2023
1 parent 43d949c commit 16e8c85
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 31 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20230506-191813.yaml
Original file line number Diff line number Diff line change
@@ -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"
3 changes: 2 additions & 1 deletion core/dbt/adapters/base/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion core/dbt/include/global_project/macros/adapters/columns.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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) }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion plugins/postgres/dbt/include/postgres/macros/adapters.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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 %}
42 changes: 40 additions & 2 deletions tests/adapter/dbt/tests/adapter/constraints/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -188,7 +198,6 @@
enforced: true
columns:
- name: id
quote: true
data_type: integer
description: hello
constraints:
Expand Down Expand Up @@ -278,7 +287,6 @@
name: strange_uniqueness_requirement
columns:
- name: id
quote: true
data_type: integer
description: hello
constraints:
Expand All @@ -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
"""
54 changes: 42 additions & 12 deletions tests/adapter/dbt/tests/adapter/constraints/test_constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)


Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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] = "<model_identifier>"
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:
Expand Down Expand Up @@ -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 = [
"<model_identifier>" 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] = "<model_identifier>"
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 <model_identifier> (
id integer not null,
"from" text not null,
date_day text,
check ("from" = 'blue')
) ;
insert into <model_identifier> (
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
);
"""

0 comments on commit 16e8c85

Please sign in to comment.