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

Type aliasing for model contract column data_type #8592

Merged
merged 19 commits into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from 17 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/Features-20230907-161831.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Features
body: Use translate_types on data_type in model.columns in templates
gshank marked this conversation as resolved.
Show resolved Hide resolved
time: 2023-09-07T16:18:31.428161-04:00
custom:
Author: gshank
Issue: "8007"
8 changes: 4 additions & 4 deletions core/dbt/adapters/base/column.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@

@dataclass
class Column:
# Note: This is automatically used by contract code
# No-op conversions (INTEGER => INT) have been removed.
# Any adapter that wants to take advantage of "translate_type"
# should create a ClassVar with the appropriate conversions.
TYPE_LABELS: ClassVar[Dict[str, str]] = {
"STRING": "TEXT",
"TIMESTAMP": "TIMESTAMP",
"FLOAT": "FLOAT",
"INTEGER": "INT",
"BOOLEAN": "BOOLEAN",
}
column: str
dtype: str
Expand Down
20 changes: 16 additions & 4 deletions core/dbt/context/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1227,12 +1227,24 @@

@contextproperty("model")
def ctx_model(self) -> Dict[str, Any]:
ret = self.model.to_dict(omit_none=True)
model_dct = self.model.to_dict(omit_none=True)
# Maintain direct use of compiled_sql
# TODO add depreciation logic[CT-934]
if "compiled_code" in ret:
ret["compiled_sql"] = ret["compiled_code"]
return ret
if "compiled_code" in model_dct:
model_dct["compiled_sql"] = model_dct["compiled_code"]

Check warning on line 1234 in core/dbt/context/providers.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/context/providers.py#L1234

Added line #L1234 was not covered by tests

if (
hasattr(self.model, "contract")
and self.model.contract.alias_types is True
and "columns" in model_dct
):
for column in model_dct["columns"].values():
if "data_type" in column:
orig_data_type = column["data_type"]

Check warning on line 1243 in core/dbt/context/providers.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/context/providers.py#L1242-L1243

Added lines #L1242 - L1243 were not covered by tests
# translate data_type to value in Column.TYPE_LABELS
new_data_type = self.adapter.Column.translate_type(orig_data_type)
Copy link
Member

Choose a reason for hiding this comment

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

What happens if the translate_type function fails? Can we do this instead?

column["data_type"] = self.adapter.Column.translate_type(orig_data_type) or column["data_type"]

Also, can we add a test for this scenario?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The base "translate_type" function return the passed in type if no translation exists. If the adapter hasn't implemented a different translate_type function it would use the base one.

column["data_type"] = new_data_type

Check warning on line 1246 in core/dbt/context/providers.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/context/providers.py#L1245-L1246

Added lines #L1245 - L1246 were not covered by tests
return model_dct

@contextproperty()
def pre_hooks(self) -> Optional[List[Dict[str, Any]]]:
Expand Down
1 change: 1 addition & 0 deletions core/dbt/contracts/graph/model_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ def default(cls) -> "OnConfigurationChangeOption":
@dataclass
class ContractConfig(dbtClassMixin, Replaceable):
enforced: bool = False
alias_types: bool = True


@dataclass
Expand Down
1 change: 1 addition & 0 deletions core/dbt/contracts/graph/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ class ColumnInfo(AdditionalPropertiesMixin, ExtensibleDbtClassMixin, Replaceable
@dataclass
class Contract(dbtClassMixin, Replaceable):
enforced: bool = False
alias_types: bool = True
checksum: Optional[str] = None


Expand Down
4 changes: 3 additions & 1 deletion core/dbt/parser/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,9 @@ def update_parsed_node_config(

# If we have contract in the config, copy to node level
if "contract" in config_dict and config_dict["contract"]:
parsed_node.contract = Contract(enforced=config_dict["contract"]["enforced"])
contract_dct = config_dict["contract"]
Contract.validate(contract_dct)
parsed_node.contract = Contract.from_dict(contract_dct)

# unrendered_config is used to compare the original database/schema/alias
# values and to handle 'same_config' and 'same_contents' calls
Expand Down
8 changes: 8 additions & 0 deletions schemas/dbt/manifest/v11.json
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,10 @@
"enforced": {
"type": "boolean",
"default": false
},
"alias_types": {
"type": "boolean",
"default": true
}
},
"additionalProperties": false
Expand Down Expand Up @@ -560,6 +564,10 @@
"type": "boolean",
"default": false
},
"alias_types": {
"type": "boolean",
"default": true
},
"checksum": {
"anyOf": [
{
Expand Down
38 changes: 19 additions & 19 deletions tests/functional/artifacts/expected_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def get_rendered_model_config(**updates):
"packages": [],
"incremental_strategy": None,
"docs": {"node_color": None, "show": True},
"contract": {"enforced": False},
"contract": {"enforced": False, "alias_types": True},
"access": "protected",
}
result.update(updates)
Expand Down Expand Up @@ -72,7 +72,7 @@ def get_rendered_seed_config(**updates):
"packages": [],
"incremental_strategy": None,
"docs": {"node_color": None, "show": True},
"contract": {"enforced": False},
"contract": {"enforced": False, "alias_types": True},
}
result.update(updates)
return result
Expand Down Expand Up @@ -112,7 +112,7 @@ def get_rendered_snapshot_config(**updates):
"packages": [],
"incremental_strategy": None,
"docs": {"node_color": None, "show": True},
"contract": {"enforced": False},
"contract": {"enforced": False, "alias_types": True},
}
result.update(updates)
return result
Expand Down Expand Up @@ -328,7 +328,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False):
"constraints": [],
},
},
"contract": {"checksum": None, "enforced": False},
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"constraints": [],
"patch_path": "test://" + model_schema_yml_path,
"docs": {"node_color": None, "show": False},
Expand Down Expand Up @@ -421,7 +421,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False):
"constraints": [],
},
},
"contract": {"checksum": None, "enforced": False},
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"constraints": [],
"patch_path": "test://" + model_schema_yml_path,
"docs": {"node_color": None, "show": False},
Expand Down Expand Up @@ -564,7 +564,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False):
},
"checksum": {"name": "none", "checksum": ""},
"unrendered_config": unrendered_test_config,
"contract": {"checksum": None, "enforced": False},
"contract": {"checksum": None, "enforced": False, "alias_types": True},
},
"snapshot.test.snapshot_seed": {
"alias": "snapshot_seed",
Expand All @@ -576,7 +576,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False):
"compiled": True,
"compiled_code": ANY,
"config": snapshot_config,
"contract": {"checksum": None, "enforced": False},
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"database": project.database,
"group": None,
"deferred": False,
Expand Down Expand Up @@ -625,7 +625,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False):
"columns": {},
"config": test_config,
"group": None,
"contract": {"checksum": None, "enforced": False},
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"sources": [],
"depends_on": {
"macros": ["macro.test.test_nothing", "macro.dbt.get_where_subquery"],
Expand Down Expand Up @@ -678,7 +678,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False):
"columns": {},
"config": test_config,
"group": None,
"contract": {"checksum": None, "enforced": False},
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"sources": [],
"depends_on": {
"macros": ["macro.dbt.test_unique", "macro.dbt.get_where_subquery"],
Expand Down Expand Up @@ -949,7 +949,7 @@ def expected_references_manifest(project):
"unique_id": "model.test.ephemeral_copy",
"compiled": True,
"compiled_code": ANY,
"contract": {"checksum": None, "enforced": False},
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"extra_ctes_injected": True,
"extra_ctes": [],
"checksum": checksum_file(ephemeral_copy_path),
Expand Down Expand Up @@ -985,7 +985,7 @@ def expected_references_manifest(project):
},
},
"config": get_rendered_model_config(materialized="table", group="test_group"),
"contract": {"checksum": None, "enforced": False},
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"sources": [],
"depends_on": {
"macros": [],
Expand Down Expand Up @@ -1054,7 +1054,7 @@ def expected_references_manifest(project):
},
},
"config": get_rendered_model_config(),
"contract": {"checksum": None, "enforced": False},
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"database": project.database,
"depends_on": {
"macros": [],
Expand Down Expand Up @@ -1178,7 +1178,7 @@ def expected_references_manifest(project):
"compiled": True,
"compiled_code": ANY,
"config": get_rendered_snapshot_config(target_schema=alternate_schema),
"contract": {"checksum": None, "enforced": False},
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"database": model_database,
"deferred": False,
"depends_on": {"macros": [], "nodes": ["seed.test.seed"]},
Expand Down Expand Up @@ -1533,7 +1533,7 @@ def expected_versions_manifest(project):
"unique_id": "model.test.versioned_model.v1",
"compiled": True,
"compiled_code": ANY,
"contract": {"checksum": None, "enforced": False},
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"extra_ctes_injected": True,
"extra_ctes": [],
"checksum": checksum_file(versioned_model_v1_path),
Expand Down Expand Up @@ -1575,7 +1575,7 @@ def expected_versions_manifest(project):
materialized="view", group="test_group", meta={"size": "large", "color": "red"}
),
"constraints": [],
"contract": {"checksum": None, "enforced": False},
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"sources": [],
"depends_on": {"macros": [], "nodes": []},
"deferred": False,
Expand Down Expand Up @@ -1622,7 +1622,7 @@ def expected_versions_manifest(project):
"columns": {},
"config": get_rendered_model_config(),
"constraints": [],
"contract": {"checksum": None, "enforced": False},
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"database": project.database,
"depends_on": {
"macros": [],
Expand Down Expand Up @@ -1683,7 +1683,7 @@ def expected_versions_manifest(project):
"columns": {},
"config": test_config,
"group": "test_group",
"contract": {"checksum": None, "enforced": False},
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"sources": [],
"depends_on": {
"macros": ["macro.dbt.test_unique", "macro.dbt.get_where_subquery"],
Expand Down Expand Up @@ -1737,7 +1737,7 @@ def expected_versions_manifest(project):
"columns": {},
"config": test_config,
"group": "test_group",
"contract": {"checksum": None, "enforced": False},
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"sources": [],
"depends_on": {
"macros": ["macro.dbt.test_unique", "macro.dbt.get_where_subquery"],
Expand Down Expand Up @@ -1791,7 +1791,7 @@ def expected_versions_manifest(project):
"columns": {},
"config": test_config,
"group": "test_group",
"contract": {"checksum": None, "enforced": False},
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"sources": [],
"depends_on": {
"macros": ["macro.dbt.test_unique", "macro.dbt.get_where_subquery"],
Expand Down
39 changes: 37 additions & 2 deletions tests/functional/configs/test_contract_configs.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,33 @@ def model(dbt, _):
tests:
- unique
- name: color
data_type: text
data_type: string
- name: date_day
data_type: date
"""

model_schema_alias_types_false_yml = """
version: 2
models:
- name: my_model
config:
contract:
enforced: true
alias_types: false
columns:
- name: id
quote: true
data_type: integer
description: hello
constraints:
- type: not_null
- type: primary_key
- type: check
expression: (id > 0)
tests:
- unique
- name: color
data_type: string
- name: date_day
data_type: date
"""
Expand Down Expand Up @@ -251,7 +277,7 @@ def test__model_contract_true(self, project):

assert contract_actual_config.enforced is True

expected_columns = "{'id': ColumnInfo(name='id', description='hello', meta={}, data_type='integer', constraints=[ColumnLevelConstraint(type=<ConstraintType.not_null: 'not_null'>, name=None, expression=None, warn_unenforced=True, warn_unsupported=True), ColumnLevelConstraint(type=<ConstraintType.primary_key: 'primary_key'>, name=None, expression=None, warn_unenforced=True, warn_unsupported=True), ColumnLevelConstraint(type=<ConstraintType.check: 'check'>, name=None, expression='(id > 0)', warn_unenforced=True, warn_unsupported=True)], quote=True, tags=[], _extra={}), 'color': ColumnInfo(name='color', description='', meta={}, data_type='text', constraints=[], quote=None, tags=[], _extra={}), 'date_day': ColumnInfo(name='date_day', description='', meta={}, data_type='date', constraints=[], quote=None, tags=[], _extra={})}"
expected_columns = "{'id': ColumnInfo(name='id', description='hello', meta={}, data_type='integer', constraints=[ColumnLevelConstraint(type=<ConstraintType.not_null: 'not_null'>, name=None, expression=None, warn_unenforced=True, warn_unsupported=True), ColumnLevelConstraint(type=<ConstraintType.primary_key: 'primary_key'>, name=None, expression=None, warn_unenforced=True, warn_unsupported=True), ColumnLevelConstraint(type=<ConstraintType.check: 'check'>, name=None, expression='(id > 0)', warn_unenforced=True, warn_unsupported=True)], quote=True, tags=[], _extra={}), 'color': ColumnInfo(name='color', description='', meta={}, data_type='string', constraints=[], quote=None, tags=[], _extra={}), 'date_day': ColumnInfo(name='date_day', description='', meta={}, data_type='date', constraints=[], quote=None, tags=[], _extra={})}"

assert expected_columns == str(my_model_columns)

Expand All @@ -264,6 +290,15 @@ def test__model_contract_true(self, project):
== cleaned_code
)

# set alias_types to false (should fail to compile)
write_file(
model_schema_alias_types_false_yml,
project.project_root,
"models",
"constraints_schema.yml",
)
run_dbt(["run"], expect_pass=False)


class TestProjectContractEnabledConfigs:
@pytest.fixture(scope="class")
Expand Down
Loading