Skip to content

Commit

Permalink
Fix ambiguous reference error for duplicate model names across packag…
Browse files Browse the repository at this point in the history
…es with tests (#8488)
  • Loading branch information
MichelleArk authored Aug 25, 2023
1 parent 1afbb87 commit 6234267
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 5 deletions.
7 changes: 7 additions & 0 deletions .changes/unreleased/Fixes-20230824-161024.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
kind: Fixes
body: fix ambiguous reference error for tests and versions when model name is duplicated across
packages
time: 2023-08-24T16:10:24.437362-04:00
custom:
Author: michelleark
Issue: "8327 8493"
2 changes: 1 addition & 1 deletion core/dbt/parser/schema_generic_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ def _lookup_attached_node(
attached_node = None # type: Optional[Union[ManifestNode, GraphMemberNode]]
if not isinstance(target, UnpatchedSourceDefinition):
attached_node_unique_id = self.manifest.ref_lookup.get_unique_id(
target.name, None, version
target.name, target.package_name, version
)
if attached_node_unique_id:
attached_node = self.manifest.nodes[attached_node_unique_id]
Expand Down
4 changes: 2 additions & 2 deletions core/dbt/parser/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ def parse_patch(self, block: TargetBlock[UnparsedModelUpdate], refs: ParserRef)
)
# ref lookup without version - version is not set yet
versioned_model_unique_id = self.manifest.ref_lookup.get_unique_id(
versioned_model_name, None, None
versioned_model_name, target.package_name, None
)

versioned_model_node = None
Expand All @@ -702,7 +702,7 @@ def parse_patch(self, block: TargetBlock[UnparsedModelUpdate], refs: ParserRef)
# If this is the latest version, it's allowed to define itself in a model file name that doesn't have a suffix
if versioned_model_unique_id is None and unparsed_version.v == latest_version:
versioned_model_unique_id = self.manifest.ref_lookup.get_unique_id(
block.name, None, None
block.name, target.package_name, None
)

if versioned_model_unique_id is None:
Expand Down
86 changes: 86 additions & 0 deletions tests/functional/duplicates/test_duplicate_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,26 @@
"""

local_dep_schema_yml = """
models:
- name: table_model
config:
alias: table_model_local_dep
columns:
- name: id
tests:
- unique
"""

local_dep_versions_schema_yml = """
models:
- name: table_model
config:
alias: table_model_local_dep
versions:
- v: 1
"""


class TestDuplicateModelEnabled:
@pytest.fixture(scope="class")
Expand Down Expand Up @@ -142,6 +162,72 @@ def test_duplicate_model_disabled_across_packages(self, project):
assert model_id in manifest.disabled


class TestDuplicateModelNameWithTestAcrossPackages:
@pytest.fixture(scope="class", autouse=True)
def setUp(self, project_root):
local_dependency_files = {
"dbt_project.yml": dbt_project_yml,
"models": {"table_model.sql": enabled_model_sql, "schema.yml": local_dep_schema_yml},
}
write_project_files(project_root, "local_dependency", local_dependency_files)

@pytest.fixture(scope="class")
def models(self):
return {"table_model.sql": enabled_model_sql}

@pytest.fixture(scope="class")
def packages(self):
return {"packages": [{"local": "local_dependency"}]}

def test_duplicate_model_name_with_test_across_packages(self, project):
run_dbt(["deps"])
manifest = run_dbt(["parse"])
assert len(manifest.nodes) == 3

# model nodes with duplicate names exist
local_dep_model_node_id = "model.local_dep.table_model"
root_model_node_id = "model.test.table_model"
assert local_dep_model_node_id in manifest.nodes
assert root_model_node_id in manifest.nodes

# test node exists and is attached to correct node
test_node_id = "test.local_dep.unique_table_model_id.1da9e464d9"
assert test_node_id in manifest.nodes
assert manifest.nodes[test_node_id].attached_node == local_dep_model_node_id


class TestDuplicateModelNameWithVersionAcrossPackages:
@pytest.fixture(scope="class", autouse=True)
def setUp(self, project_root):
local_dependency_files = {
"dbt_project.yml": dbt_project_yml,
"models": {
"table_model.sql": enabled_model_sql,
"schema.yml": local_dep_versions_schema_yml,
},
}
write_project_files(project_root, "local_dependency", local_dependency_files)

@pytest.fixture(scope="class")
def models(self):
return {"table_model.sql": enabled_model_sql}

@pytest.fixture(scope="class")
def packages(self):
return {"packages": [{"local": "local_dependency"}]}

def test_duplicate_model_name_with_test_across_packages(self, project):
run_dbt(["deps"])
manifest = run_dbt(["parse"])
assert len(manifest.nodes) == 2

# model nodes with duplicate names exist
local_dep_model_node_id = "model.local_dep.table_model.v1"
root_model_node_id = "model.test.table_model"
assert local_dep_model_node_id in manifest.nodes
assert root_model_node_id in manifest.nodes


class TestModelTestOverlap:
@pytest.fixture(scope="class")
def models(self):
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,7 @@ class SchemaParserVersionedModels(SchemaParserTest):
def setUp(self):
super().setUp()
my_model_v1_node = MockNode(
package="root",
package="snowplow",
name="arbitrary_file_name",
config=mock.MagicMock(enabled=True),
refs=[],
Expand All @@ -621,7 +621,7 @@ def setUp(self):
file_id="snowplow://models/arbitrary_file_name.sql",
)
my_model_v2_node = MockNode(
package="root",
package="snowplow",
name="my_model_v2",
config=mock.MagicMock(enabled=True),
refs=[],
Expand Down

0 comments on commit 6234267

Please sign in to comment.