Skip to content

Commit

Permalink
Fix foreign key constraints (#789)
Browse files Browse the repository at this point in the history
  • Loading branch information
benc-db authored Sep 16, 2024
1 parent 67fcb4d commit 8a2bb58
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- 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))
- Fix foreign key constraints by switching from `parent` to `to` and `parent_columns` to `to_columns` ([789](https://github.com/databricks/dbt-databricks/pull/789))

## dbt-databricks 1.8.5 (August 6, 2024)

Expand Down
4 changes: 2 additions & 2 deletions dbt/include/databricks/macros/relations/constraints.sql
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@

{% set joined_names = quoted_names|join(", ") %}

{% set parent = constraint.get("parent") %}
{% set parent = constraint.get("to") %}
{% if not parent %}
{{ exceptions.raise_compiler_error('No parent table defined for foreign key: ' ~ expression) }}
{% endif %}
Expand All @@ -218,7 +218,7 @@
{% endif %}

{% set stmt = "alter table " ~ relation ~ " add constraint " ~ name ~ " foreign key(" ~ joined_names ~ ") references " ~ parent %}
{% set parent_columns = constraint.get("parent_columns") %}
{% set parent_columns = constraint.get("to_columns") %}
{% if parent_columns %}
{% set stmt = stmt ~ "(" ~ parent_columns|join(", ") ~ ")"%}
{% endif %}
Expand Down
53 changes: 53 additions & 0 deletions tests/functional/adapter/constraints/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,56 @@
name: fk_n
expression: (n) REFERENCES {schema}.raw_numbers
"""

parent_foreign_key = """
version: 2
models:
- name: parent_table
config:
materialized: table
on_schema_change: fail
contract:
enforced: true
columns:
- name: id
data_type: integer
constraints:
- type: not_null
- type: primary_key
name: pk_example__parent_table
- name: child_table
config:
materialized: incremental
on_schema_change: fail
contract:
enforced: true
constraints:
- type: primary_key
name: pk_example__child_table
columns: ["id"]
- type: not_null
columns: ["id", "name", "parent_id"]
- type: foreign_key
name: fk_example__child_table_1
columns: ["parent_id"]
to: parent_table
to_columns: ["id"]
columns:
- name: id
data_type: integer
- name: name
data_type: string
- name: parent_id
data_type: integer
"""

parent_sql = """
select 1 as id
"""

child_sql = """
-- depends_on: {{ ref('parent_table') }}
select 2 as id, 'name' as name, 1 as parent_id
"""
16 changes: 15 additions & 1 deletion tests/functional/adapter/constraints/test_constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ def models(self):


@pytest.mark.skip_profile("databricks_cluster")
class TestIncrementalForeignKeyConstraint:
class TestIncrementalForeignKeyExpressionConstraint:
@pytest.fixture(scope="class")
def models(self):
return {
Expand All @@ -211,6 +211,20 @@ def test_incremental_foreign_key_constraint(self, project):
util.run_dbt(["run", "--select", "stg_numbers"])


@pytest.mark.skip_profile("databricks_cluster")
class TestForeignKeyParentConstraint:
@pytest.fixture(scope="class")
def models(self):
return {
"schema.yml": override_fixtures.parent_foreign_key,
"parent_table.sql": override_fixtures.parent_sql,
"child_table.sql": override_fixtures.child_sql,
}

def test_foreign_key_constraint(self, project):
util.run_dbt(["build"])


class TestConstraintQuotedColumn(BaseConstraintQuotedColumn):
@pytest.fixture(scope="class")
def models(self):
Expand Down
16 changes: 8 additions & 8 deletions tests/unit/macros/relations/test_constraint_macros.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ def test_macros_get_constraint_sql_foreign_key(self, template_bundle, model):
"type": "foreign_key",
"name": "myconstraint",
"columns": ["name"],
"parent": "parent_table",
"to": "parent_table",
}
r = self.render_constraint_sql(template_bundle, constraint, model)

Expand All @@ -346,7 +346,7 @@ def test_macros_get_constraint_sql_foreign_key_noname(self, template_bundle, mod
constraint = {
"type": "foreign_key",
"columns": ["name"],
"parent": "parent_table",
"to": "parent_table",
}
r = self.render_constraint_sql(template_bundle, constraint, model)

Expand All @@ -362,8 +362,8 @@ def test_macros_get_constraint_sql_foreign_key_parent_column(self, template_bund
"type": "foreign_key",
"name": "myconstraint",
"columns": ["name"],
"parent": "parent_table",
"parent_columns": ["parent_name"],
"to": "parent_table",
"to_columns": ["parent_name"],
}
r = self.render_constraint_sql(template_bundle, constraint, model)

Expand All @@ -379,8 +379,8 @@ def test_macros_get_constraint_sql_foreign_key_multiple_columns(self, template_b
"type": "foreign_key",
"name": "myconstraint",
"columns": ["name", "id"],
"parent": "parent_table",
"parent_columns": ["parent_name", "parent_id"],
"to": "parent_table",
"to_columns": ["parent_name", "parent_id"],
}
r = self.render_constraint_sql(template_bundle, constraint, model)

Expand All @@ -397,8 +397,8 @@ def test_macros_get_constraint_sql_foreign_key_columns_supplied_separately(
constraint = {
"type": "foreign_key",
"name": "myconstraint",
"parent": "parent_table",
"parent_columns": ["parent_name"],
"to": "parent_table",
"to_columns": ["parent_name"],
}
column = {"name": "id"}
r = self.render_constraint_sql(template_bundle, constraint, model, column)
Expand Down

0 comments on commit 8a2bb58

Please sign in to comment.