Fix constraint rendering for expressions and foreign key constraint types (#7512) (#7784)

(cherry picked from commit 05b0ebb)

Co-authored-by: Michelle Ark <[email protected]>

Author: github-actions[bot]

.changes/unreleased/Fixes-20230504-140642.yaml

@@ -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

core/dbt/adapters/base/impl.py

@@ -1318,20 +1318,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
@@ -1398,13 +1404,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:

core/dbt/contracts/graph/nodes.py

@@ -187,6 +187,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

tests/adapter/dbt/tests/adapter/constraints/fixtures.py

@@ -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(
@@ -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(
@@ -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 = """
 {{
@@ -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
@@ -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 = """
@@ -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
@@ -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
 """