diff --git a/.changes/unreleased/Fixes-20230824-161024.yaml b/.changes/unreleased/Fixes-20230824-161024.yaml new file mode 100644 index 00000000000..d39ee14ce99 --- /dev/null +++ b/.changes/unreleased/Fixes-20230824-161024.yaml @@ -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" diff --git a/core/dbt/parser/schema_generic_tests.py b/core/dbt/parser/schema_generic_tests.py index 29b71c21e92..c4994e98a74 100644 --- a/core/dbt/parser/schema_generic_tests.py +++ b/core/dbt/parser/schema_generic_tests.py @@ -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] diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index fbc95a73df7..24106a85224 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -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 @@ -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: diff --git a/tests/functional/duplicates/test_duplicate_model.py b/tests/functional/duplicates/test_duplicate_model.py index 17be1ff20b9..854bd42486d 100644 --- a/tests/functional/duplicates/test_duplicate_model.py +++ b/tests/functional/duplicates/test_duplicate_model.py @@ -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") @@ -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): diff --git a/tests/unit/test_parser.py b/tests/unit/test_parser.py index e04f93c367b..64bed3825ce 100644 --- a/tests/unit/test_parser.py +++ b/tests/unit/test_parser.py @@ -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=[], @@ -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=[],