diff --git a/CHANGELOG.md b/CHANGELOG.md index fee7c50ed..6ad3d6ad2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Fixes - Persist table comments for incremental models, snapshots and dbt clone (thanks @henlue!) ([750](https://github.com/databricks/dbt-databricks/pull/750)) +- Add relation identifier (i.e. table name) in auto generated constraint names, also adding the statement of table list for foreign keys (thanks @elca-anh!) ([774](https://github.com/databricks/dbt-databricks/pull/774)) - Update tblproperties on incremental runs. Note: only adds/edits. Deletes are too risky/complex for now ([765](https://github.com/databricks/dbt-databricks/pull/765)) - Update default scope/redirect Url for OAuth U2M, so with default OAuth app user can run python models ([776](https://github.com/databricks/dbt-databricks/pull/776)) diff --git a/dbt/include/databricks/macros/relations/constraints.sql b/dbt/include/databricks/macros/relations/constraints.sql index 91fcfa8d1..2132d4684 100644 --- a/dbt/include/databricks/macros/relations/constraints.sql +++ b/dbt/include/databricks/macros/relations/constraints.sql @@ -105,9 +105,13 @@ {% endif %} {% set name = constraint.get("name") %} - {% if not name and local_md5 %} - {{ exceptions.warn("Constraint of type " ~ type ~ " with no `name` provided. Generating hash instead.") }} - {%- set name = local_md5 (column.get("name", "") ~ ";" ~ expression ~ ";") -%} + {% if not name %} + {% if local_md5 %} + {{ exceptions.warn("Constraint of type " ~ type ~ " with no `name` provided. Generating hash instead for relation " ~ relation.identifier) }} + {%- set name = local_md5 (relation.identifier ~ ";" ~ column.get("name", "") ~ ";" ~ expression ~ ";") -%} + {% else %} + {{ exceptions.raise_compiler_error("Constraint of type " ~ type ~ " with no `name` provided, and no md5 utility.") }} + {% endif %} {% endif %} {% set stmt = "alter table " ~ relation ~ " add constraint " ~ name ~ " check (" ~ expression ~ ");" %} {% do statements.append(stmt) %} @@ -148,9 +152,13 @@ {% set joined_names = quoted_names|join(", ") %} {% set name = constraint.get("name") %} - {% if not name and local_md5 %} - {{ exceptions.warn("Constraint of type " ~ type ~ " with no `name` provided. Generating hash instead.") }} - {%- set name = local_md5("primary_key;" ~ column_names ~ ";") -%} + {% if not name %} + {% if local_md5 %} + {{ exceptions.warn("Constraint of type " ~ type ~ " with no `name` provided. Generating hash instead for relation " ~ relation.identifier) }} + {%- set name = local_md5("primary_key;" ~ relation.identifier ~ ";" ~ column_names ~ ";") -%} + {% else %} + {{ exceptions.raise_compiler_error("Constraint of type " ~ type ~ " with no `name` provided, and no md5 utility.") }} + {% endif %} {% endif %} {% set stmt = "alter table " ~ relation ~ " add constraint " ~ name ~ " primary key(" ~ joined_names ~ ");" %} {% do statements.append(stmt) %} @@ -161,12 +169,18 @@ {% endif %} {% set name = constraint.get("name") %} - {% if not name and local_md5 %} - {{ exceptions.warn("Constraint of type " ~ type ~ " with no `name` provided. Generating hash instead.") }} - {%- set name = local_md5("primary_key;" ~ column_names ~ ";") -%} - {% endif %} - + {% if constraint.get('expression') %} + + {% if not name %} + {% if local_md5 %} + {{ exceptions.warn("Constraint of type " ~ type ~ " with no `name` provided. Generating hash instead for relation " ~ relation.identifier) }} + {%- set name = local_md5("foreign_key;" ~ relation.identifier ~ ";" ~ constraint.get('expression') ~ ";") -%} + {% else %} + {{ exceptions.raise_compiler_error("Constraint of type " ~ type ~ " with no `name` provided, and no md5 utility.") }} + {% endif %} + {% endif %} + {% set stmt = "alter table " ~ relation ~ " add constraint " ~ name ~ " foreign key" ~ constraint.get('expression') %} {% else %} {% set column_names = constraint.get("columns", []) %} @@ -193,6 +207,16 @@ {% if not "." in parent %} {% set parent = relation.schema ~ "." ~ parent%} {% endif %} + + {% if not name %} + {% if local_md5 %} + {{ exceptions.warn("Constraint of type " ~ type ~ " with no `name` provided. Generating hash instead for relation " ~ relation.identifier) }} + {%- set name = local_md5("foreign_key;" ~ relation.identifier ~ ";" ~ column_names ~ ";" ~ parent ~ ";") -%} + {% else %} + {{ exceptions.raise_compiler_error("Constraint of type " ~ type ~ " with no `name` provided, and no md5 utility.") }} + {% endif %} + {% endif %} + {% set stmt = "alter table " ~ relation ~ " add constraint " ~ name ~ " foreign key(" ~ joined_names ~ ") references " ~ parent %} {% set parent_columns = constraint.get("parent_columns") %} {% if parent_columns %} diff --git a/tests/unit/macros/relations/test_constraint_macros.py b/tests/unit/macros/relations/test_constraint_macros.py index b316adbf9..0bbc3bdba 100644 --- a/tests/unit/macros/relations/test_constraint_macros.py +++ b/tests/unit/macros/relations/test_constraint_macros.py @@ -12,6 +12,11 @@ def template_name(self) -> str: def macro_folders_to_load(self) -> list: return ["macros/relations", "macros"] + @pytest.fixture(scope="class", autouse=True) + def modify_context(self, default_context) -> None: + # Mock local_md5 + default_context["local_md5"] = lambda s: f"hash({s})" + def render_constraints(self, template, *args): return self.run_macro(template, "databricks_constraints_to_dbt", *args) @@ -240,7 +245,7 @@ def test_macros_get_constraint_sql_check_named_constraint(self, template_bundle, ) assert expected in r - def test_macros_get_constraint_sql_check_none_constraint(self, template_bundle, model): + def test_macros_get_constraint_sql_check_noname_constraint(self, template_bundle, model): constraint = { "type": "check", "expression": "id != name", @@ -248,7 +253,8 @@ def test_macros_get_constraint_sql_check_none_constraint(self, template_bundle, r = self.render_constraint_sql(template_bundle, constraint, model) expected = ( - "['alter table `some_database`.`some_schema`.`some_table` add constraint None " + "['alter table `some_database`.`some_schema`.`some_table` " + "add constraint hash(some_table;;id != name;) " "check (id != name);']" ) # noqa: E501 assert expected in r @@ -307,6 +313,19 @@ def test_macros_get_constraint_sql_primary_key_with_name(self, template_bundle, ) assert expected in r + def test_macros_get_constraint_sql_primary_key_noname(self, template_bundle, model): + constraint = {"type": "primary_key"} + column = {"name": "id"} + + r = self.render_constraint_sql(template_bundle, constraint, model, column) + + expected = ( + '["alter table `some_database`.`some_schema`.`some_table` add constraint ' + "hash(primary_key;some_table;['id'];) " + 'primary key(id);"]' + ) + assert expected in r + def test_macros_get_constraint_sql_foreign_key(self, template_bundle, model): constraint = { "type": "foreign_key", @@ -323,6 +342,21 @@ def test_macros_get_constraint_sql_foreign_key(self, template_bundle, model): ) assert expected in r + def test_macros_get_constraint_sql_foreign_key_noname(self, template_bundle, model): + constraint = { + "type": "foreign_key", + "columns": ["name"], + "parent": "parent_table", + } + r = self.render_constraint_sql(template_bundle, constraint, model) + + expected = ( + '["alter table `some_database`.`some_schema`.`some_table` add ' + "constraint hash(foreign_key;some_table;['name'];some_schema.parent_table;) " + 'foreign key(name) references some_schema.parent_table;"]' + ) + assert expected in r + def test_macros_get_constraint_sql_foreign_key_parent_column(self, template_bundle, model): constraint = { "type": "foreign_key",