From 7ae4b00ba113c257e709421b56cc9fe6d937b053 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Fri, 5 Jul 2024 14:44:02 -0400 Subject: [PATCH 1/7] render to and to_columns on column and model level fk constraints --- dbt/adapters/base/impl.py | 14 ++++++++++---- pyproject.toml | 4 ++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/dbt/adapters/base/impl.py b/dbt/adapters/base/impl.py index ec5901379..f1ac2d8a1 100644 --- a/dbt/adapters/base/impl.py +++ b/dbt/adapters/base/impl.py @@ -1602,8 +1602,11 @@ def render_column_constraint(cls, constraint: ColumnLevelConstraint) -> Optional rendered_column_constraint = f"unique {constraint_expression}" elif constraint.type == ConstraintType.primary_key: rendered_column_constraint = f"primary key {constraint_expression}" - elif constraint.type == ConstraintType.foreign_key and constraint_expression: - rendered_column_constraint = f"references {constraint_expression}" + elif constraint.type == ConstraintType.foreign_key: + if constraint_expression: + rendered_column_constraint = f"references {constraint_expression}" + elif constraint.to and constraint.to_columns: + rendered_column_constraint = f"references {constraint.to}({','.join(constraint.to_columns)})" elif constraint.type == ConstraintType.custom and constraint_expression: rendered_column_constraint = constraint_expression @@ -1690,8 +1693,11 @@ def render_model_constraint(cls, constraint: ModelLevelConstraint) -> Optional[s elif constraint.type == ConstraintType.primary_key: constraint_expression = f" {constraint.expression}" if constraint.expression else "" return f"{constraint_prefix}primary key{constraint_expression} ({column_list})" - elif constraint.type == ConstraintType.foreign_key and constraint.expression: - return f"{constraint_prefix}foreign key ({column_list}) references {constraint.expression}" + elif constraint.type == ConstraintType.foreign_key: + if constraint.expression: + return f"{constraint_prefix}foreign key ({column_list}) references {constraint.expression}" + elif constraint.to and constraint.to_columns: + return f"{constraint_prefix}foreign key ({column_list}) references {constraint.to}({','.join(constraint.to_columns)})" elif constraint.type == ConstraintType.custom and constraint.expression: return f"{constraint_prefix}{constraint.expression}" else: diff --git a/pyproject.toml b/pyproject.toml index e50aa63ab..488bd659f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -57,7 +57,7 @@ include = ["dbt/adapters", "dbt/include", "dbt/__init__.py"] [tool.hatch.envs.default] dependencies = [ - "dbt_common @ git+https://github.com/dbt-labs/dbt-common.git", + "dbt_common @ git+https://github.com/dbt-labs/dbt-common.git@foreign-ref-column-constraint", 'pre-commit==3.7.0;python_version>="3.9"', 'pre-commit==3.5.0;python_version=="3.8"', ] @@ -67,7 +67,7 @@ code-quality = "pre-commit run --all-files" [tool.hatch.envs.unit-tests] dependencies = [ - "dbt_common @ git+https://github.com/dbt-labs/dbt-common.git", + "dbt_common @ git+https://github.com/dbt-labs/dbt-common.git@foreign-ref-column-constraint", "pytest", "pytest-dotenv", "pytest-xdist", From db46ba3105e7d4ef41d468150b33b5d659ba0e82 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 22 Jul 2024 12:40:42 -0400 Subject: [PATCH 2/7] update base fk test --- dbt-tests-adapter/dbt/tests/adapter/constraints/fixtures.py | 3 ++- dbt/adapters/base/impl.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/dbt-tests-adapter/dbt/tests/adapter/constraints/fixtures.py b/dbt-tests-adapter/dbt/tests/adapter/constraints/fixtures.py index cfbd53796..2a4f089bc 100644 --- a/dbt-tests-adapter/dbt/tests/adapter/constraints/fixtures.py +++ b/dbt-tests-adapter/dbt/tests/adapter/constraints/fixtures.py @@ -363,7 +363,8 @@ - type: check expression: id >= 1 - type: foreign_key - expression: {schema}.foreign_key_model (id) + to: ref('foreign_key_model') + to_columns: ["id"] - type: unique data_tests: - unique diff --git a/dbt/adapters/base/impl.py b/dbt/adapters/base/impl.py index f1ac2d8a1..b6fc84f4d 100644 --- a/dbt/adapters/base/impl.py +++ b/dbt/adapters/base/impl.py @@ -1606,7 +1606,7 @@ def render_column_constraint(cls, constraint: ColumnLevelConstraint) -> Optional if constraint_expression: rendered_column_constraint = f"references {constraint_expression}" elif constraint.to and constraint.to_columns: - rendered_column_constraint = f"references {constraint.to}({','.join(constraint.to_columns)})" + rendered_column_constraint = f"references {constraint.to} ({','.join(constraint.to_columns)})" elif constraint.type == ConstraintType.custom and constraint_expression: rendered_column_constraint = constraint_expression @@ -1697,7 +1697,7 @@ def render_model_constraint(cls, constraint: ModelLevelConstraint) -> Optional[s if constraint.expression: return f"{constraint_prefix}foreign key ({column_list}) references {constraint.expression}" elif constraint.to and constraint.to_columns: - return f"{constraint_prefix}foreign key ({column_list}) references {constraint.to}({','.join(constraint.to_columns)})" + return f"{constraint_prefix}foreign key ({column_list}) references {constraint.to} ({','.join(constraint.to_columns)})" elif constraint.type == ConstraintType.custom and constraint.expression: return f"{constraint_prefix}{constraint.expression}" else: From cd2738ce9b38e40ef8ad413ad3176d60b747a09d Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 22 Jul 2024 16:26:35 -0400 Subject: [PATCH 3/7] refactor to make mypy happy with return statement --- dbt/adapters/base/impl.py | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/dbt/adapters/base/impl.py b/dbt/adapters/base/impl.py index b6fc84f4d..b7b812954 100644 --- a/dbt/adapters/base/impl.py +++ b/dbt/adapters/base/impl.py @@ -1606,7 +1606,9 @@ def render_column_constraint(cls, constraint: ColumnLevelConstraint) -> Optional if constraint_expression: rendered_column_constraint = f"references {constraint_expression}" elif constraint.to and constraint.to_columns: - rendered_column_constraint = f"references {constraint.to} ({','.join(constraint.to_columns)})" + rendered_column_constraint = ( + f"references {constraint.to} ({','.join(constraint.to_columns)})" + ) elif constraint.type == ConstraintType.custom and constraint_expression: rendered_column_constraint = constraint_expression @@ -1685,23 +1687,29 @@ def render_model_constraint(cls, constraint: ModelLevelConstraint) -> Optional[s rendering.""" constraint_prefix = f"constraint {constraint.name} " if constraint.name else "" column_list = ", ".join(constraint.columns) + rendered_model_constraint = None + if constraint.type == ConstraintType.check and constraint.expression: - return f"{constraint_prefix}check ({constraint.expression})" + rendered_model_constraint = f"{constraint_prefix}check ({constraint.expression})" elif constraint.type == ConstraintType.unique: constraint_expression = f" {constraint.expression}" if constraint.expression else "" - return f"{constraint_prefix}unique{constraint_expression} ({column_list})" + rendered_model_constraint = ( + f"{constraint_prefix}unique{constraint_expression} ({column_list})" + ) elif constraint.type == ConstraintType.primary_key: constraint_expression = f" {constraint.expression}" if constraint.expression else "" - return f"{constraint_prefix}primary key{constraint_expression} ({column_list})" + rendered_model_constraint = ( + f"{constraint_prefix}primary key{constraint_expression} ({column_list})" + ) elif constraint.type == ConstraintType.foreign_key: if constraint.expression: - return f"{constraint_prefix}foreign key ({column_list}) references {constraint.expression}" + rendered_model_constraint = f"{constraint_prefix}foreign key ({column_list}) references {constraint.expression}" elif constraint.to and constraint.to_columns: - return f"{constraint_prefix}foreign key ({column_list}) references {constraint.to} ({','.join(constraint.to_columns)})" + rendered_model_constraint = f"{constraint_prefix}foreign key ({column_list}) references {constraint.to} ({','.join(constraint.to_columns)})" elif constraint.type == ConstraintType.custom and constraint.expression: - return f"{constraint_prefix}{constraint.expression}" - else: - return None + rendered_model_constraint = f"{constraint_prefix}{constraint.expression}" + + return rendered_model_constraint @classmethod def capabilities(cls) -> CapabilityDict: From b17ea4f0dccfa68e587a41d4b6dc2504ccd7adca Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 22 Jul 2024 16:42:19 -0400 Subject: [PATCH 4/7] unit tests and formatting tweaks --- dbt/adapters/base/impl.py | 4 ++-- tests/unit/test_base_adapter.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/dbt/adapters/base/impl.py b/dbt/adapters/base/impl.py index b7b812954..d1c0f29d1 100644 --- a/dbt/adapters/base/impl.py +++ b/dbt/adapters/base/impl.py @@ -1607,7 +1607,7 @@ def render_column_constraint(cls, constraint: ColumnLevelConstraint) -> Optional rendered_column_constraint = f"references {constraint_expression}" elif constraint.to and constraint.to_columns: rendered_column_constraint = ( - f"references {constraint.to} ({','.join(constraint.to_columns)})" + f"references {constraint.to} ({', '.join(constraint.to_columns)})" ) elif constraint.type == ConstraintType.custom and constraint_expression: rendered_column_constraint = constraint_expression @@ -1705,7 +1705,7 @@ def render_model_constraint(cls, constraint: ModelLevelConstraint) -> Optional[s if constraint.expression: rendered_model_constraint = f"{constraint_prefix}foreign key ({column_list}) references {constraint.expression}" elif constraint.to and constraint.to_columns: - rendered_model_constraint = f"{constraint_prefix}foreign key ({column_list}) references {constraint.to} ({','.join(constraint.to_columns)})" + rendered_model_constraint = f"{constraint_prefix}foreign key ({column_list}) references {constraint.to} ({', '.join(constraint.to_columns)})" elif constraint.type == ConstraintType.custom and constraint.expression: rendered_model_constraint = f"{constraint_prefix}{constraint.expression}" diff --git a/tests/unit/test_base_adapter.py b/tests/unit/test_base_adapter.py index 95fe5ae2b..64f75bc4b 100644 --- a/tests/unit/test_base_adapter.py +++ b/tests/unit/test_base_adapter.py @@ -39,6 +39,10 @@ def connection_manager(self): [{"type": "foreign_key", "expression": "other_table (c1)"}], ["column_name integer references other_table (c1)"], ), + ( + [{"type": "foreign_key", "to": "other_table", "to_columns": ["c1"]}], + ["column_name integer references other_table (c1)"], + ), ([{"type": "check"}, {"type": "unique"}], ["column_name integer unique"]), ([{"type": "custom", "expression": "-- noop"}], ["column_name integer -- noop"]), ] @@ -176,6 +180,30 @@ def test_render_raw_columns_constraints_unsupported( ], ["constraint test_name foreign key (c1, c2) references other_table (c1)"], ), + ( + [ + { + "type": "foreign_key", + "columns": ["c1", "c2"], + "to": "other_table", + "to_columns": ["c1"], + "name": "test_name", + } + ], + ["constraint test_name foreign key (c1, c2) references other_table (c1)"], + ), + ( + [ + { + "type": "foreign_key", + "columns": ["c1", "c2"], + "to": "other_table", + "to_columns": ["c1", "c2"], + "name": "test_name", + } + ], + ["constraint test_name foreign key (c1, c2) references other_table (c1, c2)"], + ), ] @pytest.mark.parametrize("constraints,expected_rendered_constraints", model_constraints) From 403f7af4bb0ba825c42cab03746c8b64ac4c7c74 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Mon, 22 Jul 2024 16:43:28 -0400 Subject: [PATCH 5/7] one more --- tests/unit/test_base_adapter.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/unit/test_base_adapter.py b/tests/unit/test_base_adapter.py index 64f75bc4b..5fa109b7e 100644 --- a/tests/unit/test_base_adapter.py +++ b/tests/unit/test_base_adapter.py @@ -43,6 +43,10 @@ def connection_manager(self): [{"type": "foreign_key", "to": "other_table", "to_columns": ["c1"]}], ["column_name integer references other_table (c1)"], ), + ( + [{"type": "foreign_key", "to": "other_table", "to_columns": ["c1", "c2"]}], + ["column_name integer references other_table (c1, c2)"], + ), ([{"type": "check"}, {"type": "unique"}], ["column_name integer unique"]), ([{"type": "custom", "expression": "-- noop"}], ["column_name integer -- noop"]), ] From 658d16350f5b22647b2e2b8ff881e6c40b7a5abe Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Tue, 23 Jul 2024 10:42:25 -0400 Subject: [PATCH 6/7] changelog entry --- .changes/unreleased/Features-20240723-104221.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Features-20240723-104221.yaml diff --git a/.changes/unreleased/Features-20240723-104221.yaml b/.changes/unreleased/Features-20240723-104221.yaml new file mode 100644 index 000000000..cc038fd47 --- /dev/null +++ b/.changes/unreleased/Features-20240723-104221.yaml @@ -0,0 +1,6 @@ +kind: Features +body: render 'to' and 'to_columns' fields on foreign key constraints +time: 2024-07-23T10:42:21.222203-04:00 +custom: + Author: michelleark + Issue: "271" From 5b27d245a0b913e09a72cd772eb9058e71948889 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Thu, 25 Jul 2024 10:21:02 -0400 Subject: [PATCH 7/7] reorder fk constraint rendering preference --- dbt/adapters/base/impl.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/dbt/adapters/base/impl.py b/dbt/adapters/base/impl.py index d1c0f29d1..e0627c472 100644 --- a/dbt/adapters/base/impl.py +++ b/dbt/adapters/base/impl.py @@ -1603,12 +1603,12 @@ def render_column_constraint(cls, constraint: ColumnLevelConstraint) -> Optional elif constraint.type == ConstraintType.primary_key: rendered_column_constraint = f"primary key {constraint_expression}" elif constraint.type == ConstraintType.foreign_key: - if constraint_expression: - rendered_column_constraint = f"references {constraint_expression}" - elif constraint.to and constraint.to_columns: + if constraint.to and constraint.to_columns: rendered_column_constraint = ( f"references {constraint.to} ({', '.join(constraint.to_columns)})" ) + elif constraint_expression: + rendered_column_constraint = f"references {constraint_expression}" elif constraint.type == ConstraintType.custom and constraint_expression: rendered_column_constraint = constraint_expression @@ -1702,10 +1702,10 @@ def render_model_constraint(cls, constraint: ModelLevelConstraint) -> Optional[s f"{constraint_prefix}primary key{constraint_expression} ({column_list})" ) elif constraint.type == ConstraintType.foreign_key: - if constraint.expression: - rendered_model_constraint = f"{constraint_prefix}foreign key ({column_list}) references {constraint.expression}" - elif constraint.to and constraint.to_columns: + if constraint.to and constraint.to_columns: rendered_model_constraint = f"{constraint_prefix}foreign key ({column_list}) references {constraint.to} ({', '.join(constraint.to_columns)})" + elif constraint.expression: + rendered_model_constraint = f"{constraint_prefix}foreign key ({column_list}) references {constraint.expression}" elif constraint.type == ConstraintType.custom and constraint.expression: rendered_model_constraint = f"{constraint_prefix}{constraint.expression}"