Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ct-2198: Unify constraints and check_constraints fields #7130

Merged
merged 22 commits into from
Mar 22, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
4d47b27
ct-2198: clean up some type names and uses
peterallenwebb Mar 6, 2023
678c4a5
CT-2198: Unify constraints and constraints_check properties on columns
peterallenwebb Mar 9, 2023
8b352de
Make mypy version consistently 0.981 (#7134)
gshank Mar 7, 2023
95020ec
CT 1808 diff based partial parsing (#6873)
gshank Mar 7, 2023
ad470e7
model contracts on models materialized as views (#7120)
emmyoop Mar 7, 2023
15c327a
Create method for env var deprecation (#7086)
stu-k Mar 8, 2023
d00eb96
update to allow adapters to change model name resolution in py models…
colin-rogers-dbt Mar 8, 2023
5945b60
add env DBT_PROJECT_DIR support #6078 (#6659)
leo-schick Mar 9, 2023
27eeb5c
Add new index.html and changelog yaml files from dbt-docs (#7141)
FishtownBuildBot Mar 10, 2023
53f1471
Make version configs optional (#7060)
dave-connors-3 Mar 10, 2023
996cfa8
[CT-1584] New top level commands: interactive compile (#7008)
aranke Mar 11, 2023
4e46095
Merge remote-tracking branch 'origin/main' into paw/ct-2198-unify-con…
peterallenwebb Mar 14, 2023
d9d9b2a
CT-2198: Add changelog entry
peterallenwebb Mar 15, 2023
f0aadb8
Merge remote-tracking branch 'origin/main' into paw/ct-2198-unify-con…
peterallenwebb Mar 15, 2023
1e2b9cc
CT-2198: Fix tests which broke after merge
peterallenwebb Mar 15, 2023
f830081
CT-2198: Add explicit validation of constraint types w/ unit test
peterallenwebb Mar 16, 2023
618f8c5
CT-2198: Move access property, per code review
peterallenwebb Mar 17, 2023
6f188e6
CT-2198: Remove a redundant macro
peterallenwebb Mar 17, 2023
70a19d3
CT-1298: Rework constraints to be adapter-generated in Python code
peterallenwebb Mar 21, 2023
674084a
Merge remote-tracking branch 'origin/main' into paw/ct-2198-unify-con…
peterallenwebb Mar 21, 2023
ec31ebd
CT-2198: Clarify function name per review
peterallenwebb Mar 22, 2023
a09f912
Merge remote-tracking branch 'origin/main' into paw/ct-2198-unify-con…
peterallenwebb Mar 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/Features-20230315-135108.yaml
Original file line number Diff line number Diff line change
@@ -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"
35 changes: 33 additions & 2 deletions core/dbt/contracts/graph/nodes.py
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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"""
Expand All @@ -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)
Expand Down
25 changes: 12 additions & 13 deletions core/dbt/contracts/graph/unparsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,23 +88,21 @@ class Docs(dbtClassMixin, Replaceable):


@dataclass
class HasDocs(AdditionalPropertiesMixin, ExtensibleDbtClassMixin, Replaceable):
class HasColumnProps(AdditionalPropertiesMixin, ExtensibleDbtClassMixin, Replaceable):
peterallenwebb marked this conversation as resolved.
Show resolved Hide resolved
peterallenwebb marked this conversation as resolved.
Show resolved Hide resolved
peterallenwebb marked this conversation as resolved.
Show resolved Hide resolved
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):
mikealfare marked this conversation as resolved.
Show resolved Hide resolved
tests: Optional[List[TestDef]] = None

def __post_init__(self):
Expand All @@ -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):
peterallenwebb marked this conversation as resolved.
Show resolved Hide resolved
columns: Sequence[UnparsedColumn] = field(default_factory=list)


Expand All @@ -145,13 +143,14 @@ class HasConfig:


@dataclass
class UnparsedAnalysisUpdate(HasConfig, HasColumnDocs, HasDocs, HasYamlMetadata):
pass
class UnparsedAnalysisUpdate(HasConfig, HasColumnDocs, HasColumnProps, HasYamlMetadata):
peterallenwebb marked this conversation as resolved.
Show resolved Hide resolved
access: Optional[str] = None


@dataclass
class UnparsedNodeUpdate(HasConfig, HasColumnTests, HasTests, HasYamlMetadata):
class UnparsedNodeUpdate(HasConfig, HasColumnTests, HasColumnAndTestProps, HasYamlMetadata):
peterallenwebb marked this conversation as resolved.
Show resolved Hide resolved
quote_columns: Optional[bool] = None
access: Optional[str] = None


@dataclass
Expand All @@ -162,7 +161,7 @@ class MacroArgument(dbtClassMixin):


@dataclass
class UnparsedMacroUpdate(HasConfig, HasDocs, HasYamlMetadata):
class UnparsedMacroUpdate(HasConfig, HasColumnProps, HasYamlMetadata):
peterallenwebb marked this conversation as resolved.
Show resolved Hide resolved
arguments: List[MacroArgument] = field(default_factory=list)


Expand Down Expand Up @@ -249,7 +248,7 @@ class Quoting(dbtClassMixin, Mergeable):


@dataclass
class UnparsedSourceTableDefinition(HasColumnTests, HasTests):
class UnparsedSourceTableDefinition(HasColumnTests, HasColumnAndTestProps):
peterallenwebb marked this conversation as resolved.
Show resolved Hide resolved
config: Dict[str, Any] = field(default_factory=dict)
loaded_at_field: Optional[str] = None
identifier: Optional[str] = None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
{% 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 }}
{{ col['name'] }} {{ col['data_type'] }} {% for x in constraints %} {{ "check" if x.type == "check" else "not null" if x.type == "not_null" else "unique" if x.type == "unique" else "primary key" if x.type == "primary_key" else "foreign key" if x.type == "foreign key" else ""
peterallenwebb marked this conversation as resolved.
Show resolved Hide resolved
}} {{ x.expression or "" }} {% endfor %} {{ "," if not loop.last }}
{% endfor %}
)
{% endmacro %}
Expand Down
15 changes: 0 additions & 15 deletions core/dbt/parser/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@
from dbt.contracts.graph.nodes import (
SourceDefinition,
Macro,
ColumnInfo,
Exposure,
Metric,
SeedNode,
Expand Down Expand Up @@ -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]]


Expand Down
6 changes: 0 additions & 6 deletions core/dbt/parser/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
38 changes: 17 additions & 21 deletions core/dbt/parser/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from dbt.contracts.graph.nodes import (
ParsedNodePatch,
ColumnInfo,
ColumnLevelConstraint,
GenericTestNode,
ParsedMacroPatch,
UnpatchedSourceDefinition,
Expand All @@ -37,11 +38,12 @@
Group,
ManifestNode,
GraphMemberNode,
ConstraintType,
)
from dbt.contracts.graph.unparsed import (
HasColumnDocs,
HasColumnTests,
HasDocs,
HasColumnProps,
SourcePatch,
UnparsedAnalysisUpdate,
UnparsedColumn,
Expand Down Expand Up @@ -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):
peterallenwebb marked this conversation as resolved.
Show resolved Hide resolved
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"])
peterallenwebb marked this conversation as resolved.
Show resolved Hide resolved
):
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],
peterallenwebb marked this conversation as resolved.
Show resolved Hide resolved
meta=column.meta,
tags=tags,
quote=quote,
_extra=column.extra,
Expand All @@ -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)
peterallenwebb marked this conversation as resolved.
Show resolved Hide resolved
return refs


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
{% for i in user_provided_columns %}
peterallenwebb marked this conversation as resolved.
Show resolved Hide resolved
{% 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 }}
{{ col['name'] }} {{ col['data_type'] }} {% for x in constraints %} {{ "check" if x.type == "check" else "not null" if x.type == "not_null" else "unique" if x.type == "unique" else "primary key" if x.type == "primary_key" else "foreign key" if x.type == "foreign key" else ""
}} {{ x.expression or "" }} {% endfor %} {{ "," if not loop.last }}
{% endfor %}
)
{% endmacro %}
Expand Down
30 changes: 19 additions & 11 deletions schemas/dbt/manifest/v9.json
Original file line number Diff line number Diff line change
Expand Up @@ -820,24 +820,32 @@
{
"type": "array",
"items": {
"type": "string"
"type": "object",
"required": ["type"],
"properties": {
"type": {
"type": "string"
},
"name": {
"type": "string"
},
"expression": {
"type": "string"
},
"warn_unenforced": {
"type": "boolean"
},
"warn_unsupported": {
"type": "boolean"
}
}
}
},
{
"type": "null"
}
]
},
"constraints_check": {
"oneOf": [
{
"type": "string"
},
{
"type": "null"
}
]
},
"quote": {
"oneOf": [
{
Expand Down
11 changes: 9 additions & 2 deletions test/unit/test_contracts_graph_parsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ def complex_parsed_model_dict():
'description': 'a text field',
'meta': {},
'tags': [],
'constraints': []
},
},
'checksum': {'name': 'sha256', 'checksum': 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'},
Expand Down Expand Up @@ -555,11 +556,13 @@ def complex_parsed_seed_dict():
'grants': {},
'docs': {'show': True},
'contract': False,
'packages': [],
'packages': [],
},
'deferred': False,
'docs': {'show': True},
'columns': {'a': {'name': 'a', 'description': 'a column description', 'meta': {}, 'tags': []}},
'columns': {
'a': {'name': 'a', 'description': 'a column description', 'meta': {}, 'tags': [], 'constraints': []}
},
'meta': {'foo': 1000},
'checksum': {'name': 'sha256', 'checksum': 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'},
'unrendered_config': {
Expand Down Expand Up @@ -696,6 +699,7 @@ def basic_parsed_model_patch_dict():
'description': 'a text field',
'meta': {},
'tags': [],
'constraints': []
},
},
'config': {},
Expand Down Expand Up @@ -904,6 +908,7 @@ def complex_parsed_hook_dict():
'description': 'a text field',
'meta': {},
'tags': [],
'constraints': [],
},
},
'index': 13,
Expand Down Expand Up @@ -1129,6 +1134,7 @@ def complex_parsed_schema_test_dict():
'description': 'a text field',
'meta': {},
'tags': [],
'constraints': []
},
},
'column_name': 'id',
Expand Down Expand Up @@ -1779,6 +1785,7 @@ def populated_parsed_node_patch_dict():
'description': 'a text field',
'meta': {},
'tags': [],
'constraints': [],
},
},
'docs': {'show': False},
Expand Down
Loading