diff --git a/.changes/unreleased/Features-20230222-130632.yaml b/.changes/unreleased/Features-20230222-130632.yaml index fd61355286c..8ed4ef89e2b 100644 --- a/.changes/unreleased/Features-20230222-130632.yaml +++ b/.changes/unreleased/Features-20230222-130632.yaml @@ -3,4 +3,4 @@ body: Enforce contracts on models materialized as tables and views time: 2023-02-22T13:06:32.583743-05:00 custom: Author: jtcohen6 michelleark emmyoop - Issue: 6751 7034 + Issue: 6751 7034 6756 diff --git a/core/dbt/context/exceptions_jinja.py b/core/dbt/context/exceptions_jinja.py index 98f19048f1a..8eb0ea09f64 100644 --- a/core/dbt/context/exceptions_jinja.py +++ b/core/dbt/context/exceptions_jinja.py @@ -23,6 +23,7 @@ PropertyYMLError, NotImplementedError, RelationWrongTypeError, + ColumnTypeMissingError, ) @@ -97,6 +98,10 @@ def relation_wrong_type(relation, expected_type, model=None) -> NoReturn: raise RelationWrongTypeError(relation, expected_type, model) +def column_type_missing(column_names) -> NoReturn: + raise ColumnTypeMissingError(column_names) + + # Update this when a new function should be added to the # dbt context's `exceptions` key! CONTEXT_EXPORTS = { @@ -119,6 +124,7 @@ def relation_wrong_type(relation, expected_type, model=None) -> NoReturn: raise_invalid_property_yml_version, raise_not_implemented, relation_wrong_type, + column_type_missing, ] } diff --git a/core/dbt/exceptions.py b/core/dbt/exceptions.py index 4159b17d4be..c1a63da11ca 100644 --- a/core/dbt/exceptions.py +++ b/core/dbt/exceptions.py @@ -1803,6 +1803,20 @@ def get_message(self) -> str: # jinja exceptions +class ColumnTypeMissingError(CompilationError): + def __init__(self, column_names: List): + self.column_names = column_names + super().__init__(msg=self.get_message()) + + def get_message(self) -> str: + msg = ( + "Contracted models require data_type to be defined for each column. " + "Please ensure that the column name and data_type are defined within " + f"the YAML configuration for the {self.column_names} column(s)." + ) + return msg + + class PatchTargetNotFoundError(CompilationError): def __init__(self, patches: Dict): self.patches = patches diff --git a/core/dbt/include/global_project/macros/adapters/columns.sql b/core/dbt/include/global_project/macros/adapters/columns.sql index 99cbf27529b..8605ab21d02 100644 --- a/core/dbt/include/global_project/macros/adapters/columns.sql +++ b/core/dbt/include/global_project/macros/adapters/columns.sql @@ -39,11 +39,18 @@ {% endmacro %} {% macro default__get_empty_schema_sql(columns) %} + {%- set col_err = [] -%} select {% for i in columns %} {%- set col = columns[i] -%} + {%- if col['data_type'] is not defined -%} + {{ col_err.append(col['name']) }} + {%- endif -%} cast(null as {{ col['data_type'] }}) as {{ col['name'] }}{{ ", " if not loop.last }} {%- endfor -%} + {%- if (col_err | length) > 0 -%} + {{ exceptions.column_type_missing(column_names=col_err) }} + {%- endif -%} {% endmacro %} {% macro get_column_schema_from_query(select_sql) -%} diff --git a/core/dbt/parser/base.py b/core/dbt/parser/base.py index fbd4c836f9d..13e925fe716 100644 --- a/core/dbt/parser/base.py +++ b/core/dbt/parser/base.py @@ -18,7 +18,7 @@ from dbt.contracts.graph.manifest import Manifest from dbt.contracts.graph.nodes import ManifestNode, BaseNode from dbt.contracts.graph.unparsed import UnparsedNode, Docs -from dbt.exceptions import DbtInternalError, ConfigUpdateError, DictParseError, ParsingError +from dbt.exceptions import DbtInternalError, ConfigUpdateError, DictParseError from dbt import hooks from dbt.node_types import NodeType, ModelLanguage from dbt.parser.search import FileBlock @@ -315,14 +315,6 @@ def update_parsed_node_config( if config_dict.get("contract", False): parsed_node.contract = True - parser_name = type(self).__name__ - if parser_name == "ModelParser": - original_file_path = parsed_node.original_file_path - error_message = "\n `contract=true` can only be configured within `schema.yml` files\n NOT within a model file(ex: .sql, .py) or `dbt_project.yml`." - raise ParsingError( - f"Original File Path: ({original_file_path})\nConstraints must be defined in a `yml` schema configuration file like `schema.yml`.\nOnly the SQL table and view materializations are supported for constraints. \n`data_type` values must be defined for all columns and NOT be null or blank.{error_message}" - ) - # unrendered_config is used to compare the original database/schema/alias # values and to handle 'same_config' and 'same_contents' calls parsed_node.unrendered_config = config.build_config_dict( diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index b5ba2262c44..7e72af8a4b4 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -949,7 +949,6 @@ def validate_constraints(self, patched_node): self.constraints_schema_validator(patched_node), self.constraints_materialization_validator(patched_node), self.constraints_language_validator(patched_node), - self.constraints_data_type_validator(patched_node), ] error_messages = [validator for validator in validators if validator != "None"] @@ -995,18 +994,6 @@ def constraints_language_validator(self, patched_node): language_error_msg_payload = f"{language_error_msg if language_error else None}" return language_error_msg_payload - def constraints_data_type_validator(self, patched_node): - data_type_errors = set() - for column, column_info in patched_node.columns.items(): - if column_info.data_type is None: - data_type_error = {column} - data_type_errors.update(data_type_error) - data_type_errors_msg = ( - f"\n Columns with `data_type` Blank/Null Errors: {data_type_errors}" - ) - data_type_errors_msg_payload = f"{data_type_errors_msg if data_type_errors else None}" - return data_type_errors_msg_payload - class TestablePatchParser(NodePatchParser[UnparsedNodeUpdate]): def get_block(self, node: UnparsedNodeUpdate) -> TestBlock: diff --git a/tests/functional/configs/test_constraint_configs.py b/tests/functional/configs/test_constraint_configs.py index 7a753eab17f..b5594b75423 100644 --- a/tests/functional/configs/test_constraint_configs.py +++ b/tests/functional/configs/test_constraint_configs.py @@ -1,6 +1,6 @@ import pytest from dbt.exceptions import ParsingError -from dbt.tests.util import run_dbt, get_manifest +from dbt.tests.util import run_dbt, get_manifest, run_dbt_and_capture my_model_sql = """ {{ @@ -133,6 +133,43 @@ def model(dbt, _): contract: true """ +model_schema_complete_datatypes_yml = """ +version: 2 +models: + - name: my_model + columns: + - name: id + quote: true + data_type: integer + description: hello + constraints: ['not null','primary key'] + constraints_check: (id > 0) + tests: + - unique + - name: color + data_type: text + - name: date_day + data_type: date +""" + +model_schema_incomplete_datatypes_yml = """ +version: 2 +models: + - name: my_model + columns: + - name: id + quote: true + data_type: integer + description: hello + constraints: ['not null','primary key'] + constraints_check: (id > 0) + tests: + - unique + - name: color + - name: date_day + data_type: date +""" + class TestModelLevelConstraintsEnabledConfigs: @pytest.fixture(scope="class") @@ -164,9 +201,6 @@ def project_config_update(self): "models": { "test": { "+contract": True, - "subdirectory": { - "+contract": False, - }, } } } @@ -175,31 +209,84 @@ def project_config_update(self): def models(self): return { "my_model.sql": my_model_sql, + "constraints_schema.yml": model_schema_complete_datatypes_yml, } - def test__project_error(self, project): - with pytest.raises(ParsingError) as err_info: - run_dbt(["parse"], expect_pass=False) + def test_defined_column_type(self, project): + run_dbt(["run"], expect_pass=True) + manifest = get_manifest(project.project_root) + model_id = "model.test.my_model" + my_model_config = manifest.nodes[model_id].config + contract_actual_config = my_model_config.contract + assert contract_actual_config is True - exc_str = " ".join(str(err_info.value).split()) - error_message_expected = "NOT within a model file(ex: .sql, .py) or `dbt_project.yml`." - assert error_message_expected in exc_str + +class TestProjectConstraintsEnabledConfigsError: + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "models": { + "test": { + "+contract": True, + } + } + } + + @pytest.fixture(scope="class") + def models(self): + return { + "my_model.sql": my_model_sql, + "constraints_schema.yml": model_schema_incomplete_datatypes_yml, + } + + def test_undefined_column_type(self, project): + results, log_output = run_dbt_and_capture(["run", "-s", "my_model"], expect_pass=False) + manifest = get_manifest(project.project_root) + model_id = "model.test.my_model" + my_model_config = manifest.nodes[model_id].config + contract_actual_config = my_model_config.contract + + assert contract_actual_config is True + + expected_compile_error = "Please ensure that the column name and data_type are defined within the YAML configuration for the ['color'] column(s)." + + assert expected_compile_error in log_output class TestModelConstraintsEnabledConfigs: + @pytest.fixture(scope="class") + def models(self): + return {"my_model.sql": my_model_contract_sql, "constraints_schema.yml": model_schema_yml} + + def test__model_contract(self, project): + run_dbt(["run"]) + manifest = get_manifest(project.project_root) + model_id = "model.test.my_model" + my_model_config = manifest.nodes[model_id].config + contract_actual_config = my_model_config.contract + assert contract_actual_config is True + + +class TestModelConstraintsEnabledConfigsMissingDataTypes: @pytest.fixture(scope="class") def models(self): return { "my_model.sql": my_model_contract_sql, + "constraints_schema.yml": model_schema_incomplete_datatypes_yml, } - def test__model_error(self, project): - with pytest.raises(ParsingError) as err_info: - run_dbt(["parse"], expect_pass=False) + def test_undefined_column_type(self, project): + results, log_output = run_dbt_and_capture(["run", "-s", "my_model"], expect_pass=False) + manifest = get_manifest(project.project_root) + model_id = "model.test.my_model" + my_model_config = manifest.nodes[model_id].config + contract_actual_config = my_model_config.contract - exc_str = " ".join(str(err_info.value).split()) - error_message_expected = "NOT within a model file(ex: .sql, .py) or `dbt_project.yml`." - assert error_message_expected in exc_str + assert contract_actual_config is True + + expected_compile_error = "Please ensure that the column name and data_type are defined within the YAML configuration for the ['color'] column(s)." + + assert expected_compile_error in log_output class TestModelLevelConstraintsDisabledConfigs: @@ -231,15 +318,16 @@ def models(self): def test__config_errors(self, project): with pytest.raises(ParsingError) as err_info: - run_dbt(["parse"], expect_pass=False) + run_dbt(["run"], expect_pass=False) exc_str = " ".join(str(err_info.value).split()) expected_materialization_error = ( "Materialization Error: {'materialization': 'Incremental'}" ) - expected_empty_data_type_error = "Columns with `data_type` Blank/Null Errors: {'date_day'}" assert expected_materialization_error in str(exc_str) - assert expected_empty_data_type_error in str(exc_str) + # This is a compile time error and we won't get here because the materialization 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 TestSchemaConstraintsEnabledConfigs: