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

[BACKPORT] Improve warning for constraints and mat types #7806

Merged
merged 3 commits into from
Jun 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/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",
]:
Comment on lines +1035 to +1037
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

materialization_enforces_constraints doesn't exist on 1.5.latest so have to hardcode this instead.

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