Skip to content

Commit

Permalink
Issue 747 primary key autoname (#774)
Browse files Browse the repository at this point in the history
Co-authored-by: Ben Cassell <98852248+benc-db@users.noreply.github.com>
Co-authored-by: Ben Cassell <ben.cassell@databricks.com>
  • Loading branch information
3 people authored Aug 20, 2024
1 parent ffa29d4 commit 55707d3
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
46 changes: 35 additions & 11 deletions dbt/include/databricks/macros/relations/constraints.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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) %}
Expand Down Expand Up @@ -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) %}
Expand All @@ -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", []) %}
Expand All @@ -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 %}
Expand Down
38 changes: 36 additions & 2 deletions tests/unit/macros/relations/test_constraint_macros.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -240,15 +245,16 @@ 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",
}
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
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down

0 comments on commit 55707d3

Please sign in to comment.