Skip to content

Commit

Permalink
Fix constraint rendering for expressions and foreign key constraint t…
Browse files Browse the repository at this point in the history
…ypes (#7512)
  • Loading branch information
MichelleArk authored Jun 2, 2023
1 parent e1d7a53 commit 05b0ebb
Show file tree
Hide file tree
Showing 6 changed files with 435 additions and 38 deletions.
7 changes: 7 additions & 0 deletions .changes/unreleased/Fixes-20230504-140642.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
kind: Fixes
body: 'Constraint rendering fixes: wrap check expression in parentheses, foreign key
''references'', support expression in all constraint types'
time: 2023-05-04T14:06:42.545193-04:00
custom:
Author: MichelleArk
Issue: 7417 7480 7416
40 changes: 24 additions & 16 deletions core/dbt/adapters/base/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -1319,20 +1319,26 @@ def _parse_column_constraint(cls, raw_constraint: Dict[str, Any]) -> ColumnLevel
def render_column_constraint(cls, constraint: ColumnLevelConstraint) -> Optional[str]:
"""Render the given constraint as DDL text. Should be overriden by adapters which need custom constraint
rendering."""
if constraint.type == ConstraintType.check and constraint.expression:
return f"check {constraint.expression}"
constraint_expression = constraint.expression or ""

rendered_column_constraint = None
if constraint.type == ConstraintType.check and constraint_expression:
rendered_column_constraint = f"check ({constraint_expression})"
elif constraint.type == ConstraintType.not_null:
return "not null"
rendered_column_constraint = f"not null {constraint_expression}"
elif constraint.type == ConstraintType.unique:
return "unique"
rendered_column_constraint = f"unique {constraint_expression}"
elif constraint.type == ConstraintType.primary_key:
return "primary key"
elif constraint.type == ConstraintType.foreign_key:
return "foreign key"
elif constraint.type == ConstraintType.custom and constraint.expression:
return constraint.expression
else:
return None
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.custom and constraint_expression:
rendered_column_constraint = constraint_expression

if rendered_column_constraint:
rendered_column_constraint = rendered_column_constraint.strip()

return rendered_column_constraint

@available
@classmethod
Expand Down Expand Up @@ -1399,13 +1405,15 @@ def render_model_constraint(cls, constraint: ModelLevelConstraint) -> Optional[s
constraint_prefix = f"constraint {constraint.name} " if constraint.name else ""
column_list = ", ".join(constraint.columns)
if constraint.type == ConstraintType.check and constraint.expression:
return f"{constraint_prefix}check {constraint.expression}"
return f"{constraint_prefix}check ({constraint.expression})"
elif constraint.type == ConstraintType.unique:
return f"{constraint_prefix}unique ({column_list})"
constraint_expression = f" {constraint.expression}" if constraint.expression else ""
return f"{constraint_prefix}unique{constraint_expression} ({column_list})"
elif constraint.type == ConstraintType.primary_key:
return f"{constraint_prefix}primary key ({column_list})"
elif constraint.type == ConstraintType.foreign_key:
return f"{constraint_prefix}foreign key ({column_list})"
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.custom and constraint.expression:
return f"{constraint_prefix}{constraint.expression}"
else:
Expand Down
3 changes: 3 additions & 0 deletions core/dbt/contracts/graph/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,9 @@ def is_valid(cls, item):
class ColumnLevelConstraint(dbtClassMixin):
type: ConstraintType
name: Optional[str] = None
# expression is a user-provided field that will depend on the constraint type.
# It could be a predicate (check type), or a sequence sql keywords (e.g. unique type),
# so the vague naming of 'expression' is intended to capture this range.
expression: Optional[str] = None
warn_unenforced: bool = (
True # Warn if constraint cannot be enforced by platform but will be in DDL
Expand Down
159 changes: 159 additions & 0 deletions tests/adapter/dbt/tests/adapter/constraints/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,17 @@
'2019-01-01' as date_day
"""

foreign_key_model_sql = """
{{
config(
materialized = "table"
)
}}
select
1 as id
"""

my_model_view_sql = """
{{
config(
Expand Down Expand Up @@ -53,6 +64,22 @@
'2019-01-01' as date_day
"""

# force dependency on foreign_key_model so that foreign key constraint is enforceable
my_model_wrong_order_depends_on_fk_sql = """
{{
config(
materialized = "table"
)
}}
-- depends_on: {{ ref('foreign_key_model') }}
select
'blue' as color,
1 as id,
'2019-01-01' as date_day
"""

my_model_view_wrong_order_sql = """
{{
config(
Expand Down Expand Up @@ -80,6 +107,24 @@
'2019-01-01' as date_day
"""

# force dependency on foreign_key_model so that foreign key constraint is enforceable
my_model_incremental_wrong_order_depends_on_fk_sql = """
{{
config(
materialized = "incremental",
on_schema_change='append_new_columns'
)
}}
-- depends_on: {{ ref('foreign_key_model') }}
select
'blue' as color,
1 as id,
'2019-01-01' as date_day
"""


# model columns name different to schema definitions
my_model_wrong_name_sql = """
{{
Expand Down Expand Up @@ -223,6 +268,95 @@
- type: primary_key
- type: check
expression: (id > 0)
- type: check
expression: id >= 1
tests:
- unique
- name: color
data_type: text
- name: date_day
data_type: text
- name: my_model_error
config:
contract:
enforced: true
columns:
- name: id
data_type: integer
description: hello
constraints:
- type: not_null
- type: primary_key
- type: check
expression: (id > 0)
tests:
- unique
- name: color
data_type: text
- name: date_day
data_type: text
- name: my_model_wrong_order
config:
contract:
enforced: true
columns:
- name: id
data_type: integer
description: hello
constraints:
- type: not_null
- type: primary_key
- type: check
expression: (id > 0)
tests:
- unique
- name: color
data_type: text
- name: date_day
data_type: text
- name: my_model_wrong_name
config:
contract:
enforced: true
columns:
- name: id
data_type: integer
description: hello
constraints:
- type: not_null
- type: primary_key
- type: check
expression: (id > 0)
tests:
- unique
- name: color
data_type: text
- name: date_day
data_type: text
"""

model_fk_constraint_schema_yml = """
version: 2
models:
- name: my_model
config:
contract:
enforced: true
columns:
- name: id
quote: true
data_type: integer
description: hello
constraints:
- type: not_null
- type: primary_key
- type: check
expression: (id > 0)
- type: check
expression: id >= 1
- type: foreign_key
expression: {schema}.foreign_key_model (id)
- type: unique
tests:
- unique
- name: color
Expand Down Expand Up @@ -286,6 +420,16 @@
data_type: text
- name: date_day
data_type: text
- name: foreign_key_model
config:
contract:
enforced: true
columns:
- name: id
data_type: integer
constraints:
- type: unique
- type: primary_key
"""

constrained_model_schema_yml = """
Expand All @@ -298,11 +442,16 @@
constraints:
- type: check
expression: (id > 0)
- type: check
expression: id >= 1
- type: primary_key
columns: [ id ]
- type: unique
columns: [ color, date_day ]
name: strange_uniqueness_requirement
- type: foreign_key
columns: [ id ]
expression: {schema}.foreign_key_model (id)
columns:
- name: id
quote: true
Expand All @@ -316,6 +465,16 @@
data_type: text
- name: date_day
data_type: text
- name: foreign_key_model
config:
contract:
enforced: true
columns:
- name: id
data_type: integer
constraints:
- type: unique
- type: primary_key
"""


Expand Down
Loading

0 comments on commit 05b0ebb

Please sign in to comment.