Skip to content

Commit

Permalink
[BACKPORT] Improve warning for constraints and mat types (#7806)
Browse files Browse the repository at this point in the history
* Improve warnings for constraints and materialization types (#7696)

* first pass

* debugging

* regen proto types

* refactor to use warn_supported flag

* PR feedback

* regen proto files after conflicts

* fix problems wqith conflict resolution
  • Loading branch information
emmyoop authored Jun 7, 2023
1 parent e5bd8b0 commit 963a38f
Show file tree
Hide file tree
Showing 7 changed files with 557 additions and 446 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20230525-073651.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Improve warnings for constraints and materialization types
time: 2023-05-25T07:36:51.855641-05:00
custom:
Author: emmyoop
Issue: "7335"
10 changes: 10 additions & 0 deletions core/dbt/events/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1182,6 +1182,16 @@ message UnpinnedRefNewVersionAvailableMsg {
UnpinnedRefNewVersionAvailable data = 2;
}

// I068
message UnsupportedConstraintMaterialization {
string materialized = 1;
}

message UnsupportedConstraintMaterializationMsg {
EventInfo info = 1;
UnsupportedConstraintMaterialization data = 2;
}

// M - Deps generation

// M001
Expand Down
13 changes: 13 additions & 0 deletions core/dbt/events/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -1148,6 +1148,19 @@ def message(self) -> str:
return msg


class UnsupportedConstraintMaterialization(WarnLevel):
def code(self):
return "I068"

def message(self) -> str:
msg = (
f"Constraint types are not supported for {self.materialized} materializations and will "
"be ignored. Set 'warn_unsupported: false' on this constraint to ignore this warning."
)

return line_wrap_message(warning_tag(msg))


# =======================================================
# M - Deps generation
# =======================================================
Expand Down
870 changes: 437 additions & 433 deletions core/dbt/events/types_pb2.py

Large diffs are not rendered by default.

35 changes: 28 additions & 7 deletions core/dbt/parser/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,13 @@
YamlParseListError,
)
from dbt.events.functions import warn_or_error
from dbt.events.types import WrongResourceSchemaFile, NoNodeForYamlKey, MacroNotFoundForPatch
from dbt.node_types import NodeType
from dbt.events.types import (
MacroNotFoundForPatch,
NoNodeForYamlKey,
UnsupportedConstraintMaterialization,
WrongResourceSchemaFile,
)
from dbt.parser.base import SimpleParser
from dbt.parser.search import FileBlock
from dbt.parser.generic_test_builders import (
Expand Down Expand Up @@ -1014,23 +1019,39 @@ def patch_constraints(self, node, constraints):
node.constraints = [ModelLevelConstraint.from_dict(c) for c in constraints]

def _validate_constraint_prerequisites(self, model_node: ModelNode):

column_warn_unsupported = [
constraint.warn_unsupported
for column in model_node.columns.values()
for constraint in column.constraints
]
model_warn_unsupported = [
constraint.warn_unsupported for constraint in model_node.constraints
]
warn_unsupported = column_warn_unsupported + model_warn_unsupported

# if any constraint has `warn_unsupported` as True then send the warning
if any(warn_unsupported) and model_node.config.materialized not in [
"table",
"incremental",
]:
warn_or_error(
UnsupportedConstraintMaterialization(materialized=model_node.config.materialized),
node=model_node,
)

errors = []
if not model_node.columns:
errors.append(
"Constraints must be defined in a `yml` schema configuration file like `schema.yml`."
)

if model_node.config.materialized not in ["table", "view", "incremental"]:
errors.append(
f"Only table, view, and incremental materializations are supported for constraints, but found '{model_node.config.materialized}'"
)

if str(model_node.language) != "sql":
errors.append(f"Language Error: Expected 'sql' but found '{model_node.language}'")

if errors:
raise ParsingError(
f"Constraint validation failed for: ({model_node.original_file_path})\n"
f"Contract enforcement failed for: ({model_node.original_file_path})\n"
+ "\n".join(errors)
)

Expand Down
68 changes: 62 additions & 6 deletions tests/functional/configs/test_contract_configs.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import pytest
import os
from dbt.exceptions import ParsingError, ValidationError
from dbt.tests.util import run_dbt, get_manifest, get_artifact, run_dbt_and_capture
from dbt.tests.util import run_dbt, get_manifest, get_artifact, run_dbt_and_capture, write_file

my_model_sql = """
{{
Expand Down Expand Up @@ -56,10 +57,10 @@
cast('2019-01-01' as date) as date_day
"""

my_ephemeral_model_sql = """
my_view_model_sql = """
{{
config(
materialized = "ephemeral"
materialized = "view"
)
}}
Expand Down Expand Up @@ -108,6 +109,34 @@ def model(dbt, _):
data_type: date
"""

model_schema_ignore_unsupported_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
warn_unsupported: False
- type: primary_key
warn_unsupported: False
- type: check
warn_unsupported: False
expression: (id > 0)
tests:
- unique
- name: color
data_type: text
- name: date_day
data_type: date
"""

model_schema_errors_yml = """
version: 2
models:
Expand Down Expand Up @@ -367,22 +396,49 @@ class TestModelLevelConstraintsErrorMessages:
@pytest.fixture(scope="class")
def models(self):
return {
"my_model.sql": my_ephemeral_model_sql,
"constraints_schema.yml": model_schema_errors_yml,
"my_model.py": my_model_python_error,
"constraints_schema.yml": model_schema_yml,
}

def test__config_errors(self, project):
with pytest.raises(ParsingError) as err_info:
run_dbt(["run"], expect_pass=False)

exc_str = " ".join(str(err_info.value).split())
expected_materialization_error = "Only table, view, and incremental materializations are supported for constraints, but found 'ephemeral'"
expected_materialization_error = "Language Error: Expected 'sql' but found 'python'"
assert expected_materialization_error in str(exc_str)
# This is a compile time error and we won't get here because the materialization check is parse time
expected_empty_data_type_error = "Columns with `data_type` Blank/Null not allowed on contracted models. Columns Blank/Null: ['date_day']"
assert expected_empty_data_type_error not in str(exc_str)


class TestModelLevelConstraintsWarningMessages:
@pytest.fixture(scope="class")
def models(self):
return {
"my_model.sql": my_view_model_sql,
"constraints_schema.yml": model_schema_yml,
}

def test__config_warning(self, project):
_, log_output = run_dbt_and_capture(["run"])

expected_materialization_warning = (
"Constraint types are not supported for view materializations"
)
assert expected_materialization_warning in str(log_output)

# change to not show warnings, message should not be in logs
models_dir = os.path.join(project.project_root, "models")
write_file(model_schema_ignore_unsupported_yml, models_dir, "constraints_schema.yml")
_, log_output = run_dbt_and_capture(["run"])

expected_materialization_warning = (
"Constraint types are not supported for view materializations"
)
assert expected_materialization_warning not in str(log_output)


class TestSchemaContractEnabledConfigs:
@pytest.fixture(scope="class")
def models(self):
Expand Down
1 change: 1 addition & 0 deletions tests/unit/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ def test_event_codes(self):
types.UnpinnedRefNewVersionAvailable(
ref_node_name="", ref_node_package="", ref_node_version="", ref_max_version=""
),
types.UnsupportedConstraintMaterialization(materialized=""),
# M - Deps generation ======================
types.GitSparseCheckoutSubdirectory(subdir=""),
types.GitProgressCheckoutRevision(revision=""),
Expand Down

0 comments on commit 963a38f

Please sign in to comment.