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.

Sign up for GitHub

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

Type aliasing for model contract column data_type #8592

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
436010e
Add "alias_types" to config contract and node contracts, fix unit tests
gshank Sep 5, 2023
f05acd1
Update artifacts test
gshank Sep 5, 2023
a593b13
Fix test_list.py output
gshank Sep 5, 2023
20032d8
Change data_type in model.columns using translate_types
gshank Sep 7, 2023
9ac3375
Changie
gshank Sep 7, 2023
396838f
Change data_type to string in functional test
gshank Sep 7, 2023
9e9bc5a
Fix use of dictionary columns
gshank Sep 7, 2023
cc86526
Fix test_contract_configs.py test
gshank Sep 7, 2023
7ecac7f
Add test for alias_types: False
gshank Sep 8, 2023
d0e46af
Fix test_constraints.py to use "int" in sql instead of "integer"
gshank Sep 8, 2023
c42f2f5
Remove unnecessary TYPE_LABELs
gshank Sep 12, 2023
053c1e1
Merge branch 'main' into 8007-contract_type_aliasing
gshank Sep 12, 2023
f9ed1fa
change back test_constraints.py
gshank Sep 12, 2023
9772598
Merge branch 'main' into 8007-contract_type_aliasing
gshank Sep 14, 2023
118d8a6
Merge branch 'main' into 8007-contract_type_aliasing
gshank Sep 26, 2023
2fe0382
Fix list test
gshank Sep 27, 2023
a44648d
Merge branch 'main' into 8007-contract_type_aliasing
gshank Oct 5, 2023
1544c10
Merge branch 'main' into 8007-contract_type_aliasing
gshank Oct 10, 2023
4084fc4
Update .changes/unreleased/Features-20230907-161831.yaml
gshank Oct 10, 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-20230907-161831.yaml
View file
Open in desktop
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"
20 changes: 16 additions & 4 deletions core/dbt/context/providers.py
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ class ColumnInfo(AdditionalPropertiesMixin, ExtensibleDbtClassMixin, Replaceable
@dataclass
class Contract(dbtClassMixin, Replaceable):
enforced: bool = False
alias_types: bool = True
checksum: Optional[str] = None


Expand Down
10 changes: 9 additions & 1 deletion schemas/dbt/manifest/v11.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
},
"dbt_version": {
"type": "string",
"default": "1.7.0b1"
"default": "1.7.0b2"
},
"generated_at": {
"type": "string"
Expand Down 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
View file
Open in desktop
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},
}
result.update(updates)
return result
Expand Down Expand Up @@ -71,7 +71,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 @@ -111,7 +111,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 @@ -327,7 +327,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 @@ -420,7 +420,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 @@ -563,7 +563,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 @@ -575,7 +575,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 @@ -624,7 +624,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 @@ -677,7 +677,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 @@ -948,7 +948,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 @@ -984,7 +984,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 @@ -1053,7 +1053,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 @@ -1177,7 +1177,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 @@ -1532,7 +1532,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 @@ -1574,7 +1574,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 @@ -1621,7 +1621,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 @@ -1682,7 +1682,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 @@ -1736,7 +1736,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 @@ -1790,7 +1790,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
4 changes: 2 additions & 2 deletions tests/functional/configs/test_contract_configs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def model(dbt, _):
tests:
- unique
- name: color
data_type: text
data_type: string
- name: date_day
data_type: date
"""
Expand Down Expand Up @@ -251,7 +251,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 Down
14 changes: 7 additions & 7 deletions tests/functional/list/test_list.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def expect_snapshot_output(self, project):
"packages": [],
"incremental_strategy": None,
"docs": {"node_color": None, "show": True},
"contract": {"enforced": False},
"contract": {"enforced": False, "alias_types": True},
},
"unique_id": "snapshot.test.my_snapshot",
"original_file_path": normalize("snapshots/snapshot.sql"),
Expand Down Expand Up @@ -138,7 +138,7 @@ def expect_analyses_output(self):
"packages": [],
"incremental_strategy": None,
"docs": {"node_color": None, "show": True},
"contract": {"enforced": False},
"contract": {"enforced": False, "alias_types": True},
},
"unique_id": "analysis.test.a",
"original_file_path": normalize("analyses/a.sql"),
Expand Down Expand Up @@ -181,7 +181,7 @@ def expect_model_output(self):
"packages": [],
"incremental_strategy": None,
"docs": {"node_color": None, "show": True},
"contract": {"enforced": False},
"contract": {"enforced": False, "alias_types": True},
},
"original_file_path": normalize("models/ephemeral.sql"),
"unique_id": "model.test.ephemeral",
Expand Down Expand Up @@ -218,7 +218,7 @@ def expect_model_output(self):
"packages": [],
"incremental_strategy": "delete+insert",
"docs": {"node_color": None, "show": True},
"contract": {"enforced": False},
"contract": {"enforced": False, "alias_types": True},
},
"original_file_path": normalize("models/incremental.sql"),
"unique_id": "model.test.incremental",
Expand Down Expand Up @@ -255,7 +255,7 @@ def expect_model_output(self):
"packages": [],
"incremental_strategy": None,
"docs": {"node_color": None, "show": True},
"contract": {"enforced": False},
"contract": {"enforced": False, "alias_types": True},
},
"original_file_path": normalize("models/sub/inner.sql"),
"unique_id": "model.test.inner",
Expand Down Expand Up @@ -292,7 +292,7 @@ def expect_model_output(self):
"packages": [],
"incremental_strategy": None,
"docs": {"node_color": None, "show": True},
"contract": {"enforced": False},
"contract": {"enforced": False, "alias_types": True},
},
"original_file_path": normalize("models/outer.sql"),
"unique_id": "model.test.outer",
Expand Down Expand Up @@ -407,7 +407,7 @@ def expect_seed_output(self):
"packages": [],
"incremental_strategy": None,
"docs": {"node_color": None, "show": True},
"contract": {"enforced": False},
"contract": {"enforced": False, "alias_types": True},
},
"depends_on": {"macros": []},
"unique_id": "seed.test.seed",
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/test_contracts_graph_compiled.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,12 @@ def basic_compiled_dict():
"meta": {},
"grants": {},
"packages": [],
"contract": {"enforced": False},
"contract": {"enforced": False, "alias_types": True},
"docs": {"show": True},
},
"docs": {"show": True},
"columns": {},
"contract": {"enforced": False},
"contract": {"enforced": False, "alias_types": True},
"meta": {},
"compiled": True,
"extra_ctes": [{"id": "whatever", "sql": "select * from other"}],
Expand Down Expand Up @@ -553,7 +553,7 @@ def basic_compiled_schema_test_dict():
},
"docs": {"show": True},
"columns": {},
"contract": {"enforced": False},
"contract": {"enforced": False, "alias_types": True},
"meta": {},
"compiled": True,
"extra_ctes": [{"id": "whatever", "sql": "select * from other"}],
Expand Down
Loading