ct-2198: Unify constraints and check_constraints fields (#7130)

* ct-2198: clean up some type names and uses * CT-2198: Unify constraints and constraints_check properties on columns * Make mypy version consistently 0.981 (#7134) * CT 1808 diff based partial parsing (#6873) * model contracts on models materialized as views (#7120) * first pass * rename tests * fix failing test * changelog * fix functional test * Update core/dbt/parser/base.py * Update core/dbt/parser/schemas.py * Create method for env var deprecation (#7086) * update to allow adapters to change model name resolution in py models (#7115) * update to allow adapters to change model name resolution in py models * add changie * fix newline adds * move quoting into macro * use single quotes * add env DBT_PROJECT_DIR support #6078 (#6659) Co-authored-by: Jeremy Cohen <[email protected]> * Add new index.html and changelog yaml files from dbt-docs (#7141) * Make version configs optional (#7060) * [CT-1584] New top level commands: interactive compile (#7008) Co-authored-by: Github Build Bot <[email protected]> * CT-2198: Add changelog entry * CT-2198: Fix tests which broke after merge * CT-2198: Add explicit validation of constraint types w/ unit test * CT-2198: Move access property, per code review * CT-2198: Remove a redundant macro * CT-1298: Rework constraints to be adapter-generated in Python code * CT-2198: Clarify function name per review --------- Co-authored-by: Gerda Shank <[email protected]> Co-authored-by: Emily Rockman <[email protected]> Co-authored-by: Stu Kilgore <[email protected]> Co-authored-by: colin-rogers-dbt <[email protected]> Co-authored-by: Leo Schick <[email protected]> Co-authored-by: Jeremy Cohen <[email protected]> Co-authored-by: FishtownBuildBot <[email protected]> Co-authored-by: dave-connors-3 <[email protected]> Co-authored-by: Kshitij Aranke <[email protected]> Co-authored-by: Github Build Bot <[email protected]>
dbt-labs · Mar 22, 2023 · 73ff497 · 73ff497
1 parent 9a7305d
commit 73ff497
Show file tree

Hide file tree

Showing 19 changed files with 250 additions and 183 deletions.
diff --git a/.changes/unreleased/Features-20230315-135108.yaml b/.changes/unreleased/Features-20230315-135108.yaml
@@ -0,0 +1,6 @@
+kind: Features
+body: Unified constraints and check_constraints properties for columns and models
+time: 2023-03-15T13:51:08.259624-04:00
+custom:
+  Author: peterallenwebb
+  Issue: "7066"
diff --git a/core/dbt/adapters/base/impl.py b/core/dbt/adapters/base/impl.py
@@ -5,37 +5,40 @@
 import time
 from itertools import chain
 from typing import (
-    Optional,
-    Tuple,
+    Any,
     Callable,
-    Iterable,
-    Type,
     Dict,
-    Any,
+    Iterable,
+    Iterator,
     List,
     Mapping,
-    Iterator,
+    Optional,
     Set,
+    Tuple,
+    Type,
 )
 
+from dbt.contracts.graph.nodes import ColumnLevelConstraint, ConstraintType
+
 import agate
 import pytz
 
 from dbt.exceptions import (
     DbtInternalError,
+    DbtRuntimeError,
+    DbtValidationError,
     MacroArgTypeError,
     MacroResultError,
-    QuoteConfigTypeError,
     NotImplementedError,
     NullRelationCacheAttemptedError,
     NullRelationDropAttemptedError,
+    QuoteConfigTypeError,
     RelationReturnedMultipleResultsError,
     RenameToNoneAttemptedError,
-    DbtRuntimeError,
     SnapshotTargetIncompleteError,
     SnapshotTargetNotSnapshotTableError,
-    UnexpectedNullError,
     UnexpectedNonTimestampError,
+    UnexpectedNullError,
 )
 
 from dbt.adapters.protocol import AdapterConfig, ConnectionManagerProtocol
@@ -1262,6 +1265,39 @@ def get_incremental_strategy_macro(self, model_context, strategy: str):
         # This returns a callable macro
         return model_context[macro_name]
 
+    @classmethod
+    def _parse_column_constraint(cls, raw_constraint: Dict[str, Any]) -> ColumnLevelConstraint:
+        try:
+            ColumnLevelConstraint.validate(raw_constraint)
+            return ColumnLevelConstraint.from_dict(raw_constraint)
+        except Exception:
+            raise DbtValidationError(f"Could not parse constraint: {raw_constraint}")
+
+    @available
+    @classmethod
+    def render_raw_column_constraint(cls, raw_constraint: Dict[str, Any]) -> str:
+        constraint = cls._parse_column_constraint(raw_constraint)
+        return cls.render_column_constraint(constraint)
+
+    @classmethod
+    def render_column_constraint(cls, constraint: ColumnLevelConstraint) -> 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}"
+        elif constraint.type == ConstraintType.not_null:
+            return "not null"
+        elif constraint.type == ConstraintType.unique:
+            return "unique"
+        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 ""
+
 
 COLUMNS_EQUAL_SQL = """
 with diff_count as (

diff --git a/core/dbt/contracts/graph/nodes.py b/core/dbt/contracts/graph/nodes.py
@@ -1,6 +1,8 @@
 import os
 import time
 from dataclasses import dataclass, field
+from enum import Enum
+
 from mashumaro.types import SerializableType
 from typing import (
     Optional,
@@ -140,6 +142,36 @@ def same_fqn(self, other) -> bool:
         return self.fqn == other.fqn
 
 
+class ConstraintType(str, Enum):
+    check = "check"
+    not_null = "not_null"
+    unique = "unique"
+    primary_key = "primary_key"
+    foreign_key = "foreign_key"
+    custom = "custom"
+
+    @classmethod
+    def is_valid(cls, item):
+        try:
+            cls(item)
+        except ValueError:
+            return False
+        return True
+
+
+@dataclass
+class ColumnLevelConstraint(dbtClassMixin):
+    type: ConstraintType
+    name: Optional[str] = None
+    expression: Optional[str] = None
+    warn_unenforced: bool = (
+        True  # Warn if constraint cannot be enforced by platform but will be in DDL
+    )
+    warn_unsupported: bool = (
+        True  # Warn if constraint is not supported by the platform and won't be in DDL
+    )
+
+
 @dataclass
 class ColumnInfo(AdditionalPropertiesMixin, ExtensibleDbtClassMixin, Replaceable):
     """Used in all ManifestNodes and SourceDefinition"""
@@ -148,8 +180,7 @@ class ColumnInfo(AdditionalPropertiesMixin, ExtensibleDbtClassMixin, Replaceable
     description: str = ""
     meta: Dict[str, Any] = field(default_factory=dict)
     data_type: Optional[str] = None
-    constraints: Optional[List[str]] = None
-    constraints_check: Optional[str] = None
+    constraints: List[ColumnLevelConstraint] = field(default_factory=list)
     quote: Optional[bool] = None
     tags: List[str] = field(default_factory=list)
     _extra: Dict[str, Any] = field(default_factory=dict)

diff --git a/core/dbt/contracts/graph/unparsed.py b/core/dbt/contracts/graph/unparsed.py
@@ -88,23 +88,21 @@ class Docs(dbtClassMixin, Replaceable):
 
 
 @dataclass
-class HasDocs(AdditionalPropertiesMixin, ExtensibleDbtClassMixin, Replaceable):
+class HasColumnProps(AdditionalPropertiesMixin, ExtensibleDbtClassMixin, Replaceable):
     name: str
     description: str = ""
     meta: Dict[str, Any] = field(default_factory=dict)
     data_type: Optional[str] = None
-    constraints: Optional[List[str]] = None
-    constraints_check: Optional[str] = None
+    constraints: List[Dict[str, Any]] = field(default_factory=list)
     docs: Docs = field(default_factory=Docs)
-    access: Optional[str] = None
     _extra: Dict[str, Any] = field(default_factory=dict)
 
 
 TestDef = Union[Dict[str, Any], str]
 
 
 @dataclass
-class HasTests(HasDocs):
+class HasColumnAndTestProps(HasColumnProps):
     tests: Optional[List[TestDef]] = None
 
     def __post_init__(self):
@@ -113,18 +111,18 @@ def __post_init__(self):
 
 
 @dataclass
-class UnparsedColumn(HasTests):
+class UnparsedColumn(HasColumnAndTestProps):
     quote: Optional[bool] = None
     tags: List[str] = field(default_factory=list)
 
 
 @dataclass
 class HasColumnDocs(dbtClassMixin, Replaceable):
-    columns: Sequence[HasDocs] = field(default_factory=list)
+    columns: Sequence[HasColumnProps] = field(default_factory=list)
 
 
 @dataclass
-class HasColumnTests(HasColumnDocs):
+class HasColumnTests(dbtClassMixin, Replaceable):
     columns: Sequence[UnparsedColumn] = field(default_factory=list)
 
 
@@ -145,13 +143,14 @@ class HasConfig:
 
 
 @dataclass
-class UnparsedAnalysisUpdate(HasConfig, HasColumnDocs, HasDocs, HasYamlMetadata):
-    pass
+class UnparsedAnalysisUpdate(HasConfig, HasColumnDocs, HasColumnProps, HasYamlMetadata):
+    access: Optional[str] = None
 
 
 @dataclass
-class UnparsedNodeUpdate(HasConfig, HasColumnTests, HasTests, HasYamlMetadata):
+class UnparsedNodeUpdate(HasConfig, HasColumnTests, HasColumnAndTestProps, HasYamlMetadata):
     quote_columns: Optional[bool] = None
+    access: Optional[str] = None
 
 
 @dataclass
@@ -162,7 +161,7 @@ class MacroArgument(dbtClassMixin):
 
 
 @dataclass
-class UnparsedMacroUpdate(HasConfig, HasDocs, HasYamlMetadata):
+class UnparsedMacroUpdate(HasConfig, HasColumnProps, HasYamlMetadata):
     arguments: List[MacroArgument] = field(default_factory=list)
 
 
@@ -249,7 +248,7 @@ class Quoting(dbtClassMixin, Mergeable):
 
 
 @dataclass
-class UnparsedSourceTableDefinition(HasColumnTests, HasTests):
+class UnparsedSourceTableDefinition(HasColumnTests, HasColumnAndTestProps):
     config: Dict[str, Any] = field(default_factory=dict)
     loaded_at_field: Optional[str] = None
     identifier: Optional[str] = None

diff --git a/core/dbt/include/global_project/macros/materializations/models/table/columns_spec_ddl.sql b/core/dbt/include/global_project/macros/materializations/models/table/columns_spec_ddl.sql
@@ -11,12 +11,11 @@
     {%- set user_provided_columns = model['columns'] -%}
     (
     {% for i in user_provided_columns %}
-      {% set col = user_provided_columns[i] %}
-      {% set constraints = col['constraints'] %}
-      {% set constraints_check = col['constraints_check'] %}
-      {{ col['name'] }} {{ col['data_type'] }} {% for x in constraints %} {{ x or "" }} {% endfor %} {% if constraints_check -%} check {{ constraints_check or "" }} {%- endif %} {{ "," if not loop.last }}
-    {% endfor %}
-  )
+      {%- set col = user_provided_columns[i] -%}
+      {%- set constraints = col['constraints'] -%}
+      {{ col['name'] }} {{ col['data_type'] }}{% for c in constraints %} {{ adapter.render_raw_column_constraint(c) }}{% endfor %}{{ "," if not loop.last }}
+    {% endfor -%}
+    )
 {% endmacro %}
 
 {%- macro get_assert_columns_equivalent(sql) -%}

diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py
@@ -64,7 +64,6 @@
 from dbt.contracts.graph.nodes import (
     SourceDefinition,
     Macro,
-    ColumnInfo,
     Exposure,
     Metric,
     SeedNode,
@@ -1130,20 +1129,6 @@ def _check_manifest(manifest: Manifest, config: RuntimeConfig) -> None:
     _warn_for_unused_resource_config_paths(manifest, config)
 
 
-def _get_node_column(node, column_name):
-    """Given a ManifestNode, add some fields that might be missing. Return a
-    reference to the dict that refers to the given column, creating it if
-    it doesn't yet exist.
-    """
-    if column_name in node.columns:
-        column = node.columns[column_name]
-    else:
-        node.columns[column_name] = ColumnInfo(name=column_name)
-        node.columns[column_name] = column
-
-    return column
-
-
 DocsContextCallback = Callable[[ResultNode], Dict[str, Any]]
 
 

diff --git a/core/dbt/parser/models.py b/core/dbt/parser/models.py
@@ -152,12 +152,6 @@ def visit_ImportFrom(self, node: ast.ImportFrom) -> None:
             self.packages.append(node.module.split(".")[0])
 
 
-def merge_packages(original_packages_with_version, new_packages):
-    original_packages = [package.split("==")[0] for package in original_packages_with_version]
-    additional_packages = [package for package in new_packages if package not in original_packages]
-    return original_packages_with_version + list(set(additional_packages))
-
-
 def verify_python_model_code(node):
     # TODO: add a test for this
     try:

diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py
@@ -29,6 +29,7 @@
 from dbt.contracts.graph.nodes import (
     ParsedNodePatch,
     ColumnInfo,
+    ColumnLevelConstraint,
     GenericTestNode,
     ParsedMacroPatch,
     UnpatchedSourceDefinition,
@@ -37,11 +38,12 @@
     Group,
     ManifestNode,
     GraphMemberNode,
+    ConstraintType,
 )
 from dbt.contracts.graph.unparsed import (
     HasColumnDocs,
     HasColumnTests,
-    HasDocs,
+    HasColumnProps,
     SourcePatch,
     UnparsedAnalysisUpdate,
     UnparsedColumn,
@@ -114,29 +116,28 @@ class ParserRef:
     def __init__(self):
         self.column_info: Dict[str, ColumnInfo] = {}
 
-    def add(
-        self,
-        column: Union[HasDocs, UnparsedColumn],
-        description: str,
-        data_type: Optional[str],
-        constraints: Optional[List[str]],
-        constraints_check: Optional[str],
-        meta: Dict[str, Any],
-    ):
+    def _add(self, column: HasColumnProps):
         tags: List[str] = []
         tags.extend(getattr(column, "tags", ()))
         quote: Optional[bool]
         if isinstance(column, UnparsedColumn):
             quote = column.quote
         else:
             quote = None
+
+        if any(
+            c
+            for c in column.constraints
+            if not c["type"] or not ConstraintType.is_valid(c["type"])
+        ):
+            raise ParsingError(f"Invalid constraint type on column {column.name}")
+
         self.column_info[column.name] = ColumnInfo(
             name=column.name,
-            description=description,
-            data_type=data_type,
-            constraints=constraints,
-            constraints_check=constraints_check,
-            meta=meta,
+            description=column.description,
+            data_type=column.data_type,
+            constraints=[ColumnLevelConstraint.from_dict(c) for c in column.constraints],
+            meta=column.meta,
             tags=tags,
             quote=quote,
             _extra=column.extra,
@@ -146,12 +147,7 @@ def add(
     def from_target(cls, target: Union[HasColumnDocs, HasColumnTests]) -> "ParserRef":
         refs = cls()
         for column in target.columns:
-            description = column.description
-            data_type = column.data_type
-            constraints = column.constraints
-            constraints_check = column.constraints_check
-            meta = column.meta
-            refs.add(column, description, data_type, constraints, constraints_check, meta)
+            refs._add(column)
         return refs
 
 

diff --git a/plugins/postgres/dbt/include/postgres/macros/utils/columns_spec_ddl.sql b/plugins/postgres/dbt/include/postgres/macros/utils/columns_spec_ddl.sql
@@ -1,16 +1,3 @@
-{% macro postgres__get_columns_spec_ddl() %}
-  {# loop through user_provided_columns to create DDL with data types and constraints #}
-    {%- set user_provided_columns = model['columns'] -%}
-    (
-    {% for i in user_provided_columns %}
-      {% set col = user_provided_columns[i] %}
-      {% set constraints = col['constraints'] %}
-      {% set constraints_check = col['constraints_check'] %}
-      {{ col['name'] }} {{ col['data_type'] }} {% for x in constraints %} {{ x or "" }} {% endfor %} {% if constraints_check -%} check {{ constraints_check or "" }} {%- endif %} {{ "," if not loop.last }}
-    {% endfor %}
-  )
-{% endmacro %}
-
 {% macro get_column_names() %}
   {# loop through user_provided_columns to get column names #}
     {%- set user_provided_columns = model['columns'] -%}