Repository: dbt-labs/dbt-core
Pull request: CT 2510 Throw error for duplicate versioned and non versioned model names
resolves #7487

### Description

The _check_resource_uniqueness method in core/dbt/parser/manifest.py has been modified to check for non-versioned models with the same "name" as versioned models.

### Checklist

- [x] I have read [the contributing guide](https://github.com/dbt-labs/dbt-core/blob/main/CONTRIBUTING.md) and understand what's expected of me
- [x] I have signed the [CLA](https://docs.getdbt.com/docs/contributor-license-agreements)
- [x] I have run this code in development and it appears to resolve the stated issue
- [x] This PR includes tests, or tests are not required/relevant for this PR
- [x] I have [opened an issue to add/update docs](https://github.com/dbt-labs/docs.getdbt.com/issues/new/choose), or docs changes are not required/relevant for this PR
- [x] I have run `changie new` to [create a changelog entry](https://github.com/dbt-labs/dbt-core/blob/main/CONTRIBUTING.md#adding-a-changelog-entry)


Problem statement:
[CT-2510] [Bug] Raise an error if non-versioned model is defined with same name as existing versioned model
Set up:
```sql
-- models/my_model_v1.sql
select 1 as id
```
```yml
# models/whatever.yml
models:
  - name: my_model
    versions:
      - v: 1
```

```
$ dbt parse
12:48:12  Running with dbt=1.5.0
12:48:14  Performance info: target/perf_info.json
```
Now:
- create `models/my_model.sql` (no version suffix)
- do not add any new versions to the `versions` block in the yaml file

Re-parse with partial parsing, and everything looks fine:
```
$ dbt parse
12:48:29  Running with dbt=1.5.0
12:48:30  Performance info: target/perf_info.json
```
And now without partial parsing, I see an error (good! I should!):
```
$ dbt --no-partial-parse parse
12:48:43  Running with dbt=1.5.0
12:48:46  Encountered an error:
Compilation Error
  dbt found two models with the name "my_model".

  Since these resources have the same name, dbt will be unable to find the correct resource
  when looking for ref("my_model").

  To fix this, change the name of one of these resources:
  - model.my_dbt_project.my_model (models/my_model.sql)
  - model.my_dbt_project.my_model.v1 (models/my_model_v1.sql)
```

Hints from issue discussion:
This is not a partial parsing bug per se. I've created a test case which does not fail appropriately even with full parsing.
I haven't been able to recreate the compilation error. Whether I create the non-versioned model first or second, it never complains.
I tried the same set of steps as outlined and was **not** able to produce the compilation error.
I **can** reproduce this now. I accidentally didn't create and save `models/my_model.sql` 😅 

Here's my key files at the end:

`models/_models.yml`
```yaml
models:
  - name: my_model
    versions:
      - v: 1
```

`models/my_model_v1.sql`
```sql
select 1 as id
```

`models/my_model.sql`
```sql
select 0 as id
```
Commit 0de046dfbef53cfe754c87ec58a638c7a8962dc6 yesterday removed the code that was causing the error in the example to be thrown.
The error was thrown in the no-partial-parse case and not in the partial parse case because the order of the nodes encountered was different and the code was checking is_versioned for the previously encountered node.

Commit message: Changie

Patch to review:
diff --git a/.changes/unreleased/Fixes-20230509-165007.yaml b/.changes/unreleased/Fixes-20230509-165007.yaml
new file mode 100644
index 00000000000..0c0f06b0525
--- /dev/null
+++ b/.changes/unreleased/Fixes-20230509-165007.yaml
@@ -0,0 +1,6 @@
+kind: Fixes
+body: Throw error for duplicated versioned and unversioned models
+time: 2023-05-09T16:50:07.530882-04:00
+custom:
+  Author: gshank
+  Issue: "7487"
diff --git a/core/dbt/exceptions.py b/core/dbt/exceptions.py
index e92948d9ad0..a50a0993b48 100644
--- a/core/dbt/exceptions.py
+++ b/core/dbt/exceptions.py
@@ -2187,6 +2187,25 @@ def get_message(self) -> str:
         return msg
 
 
+class DuplicateVersionedUnversionedError(ParsingError):
+    def __init__(self, versioned_node, unversioned_node):
+        self.versioned_node = versioned_node
+        self.unversioned_node = unversioned_node
+        super().__init__(msg=self.get_message())
+
+    def get_message(self) -> str:
+        msg = f"""
+dbt found versioned and unversioned models with the name "{self.versioned_node.name}".
+
+Since these resources have the same name, dbt will be unable to find the correct resource
+when looking for ref({self.versioned_node.name}).
+
+To fix this, change the name of the unversioned resource:
+- {self.unversioned_node.unique_id} ({self.unversioned_node.original_file_path})
+    """.strip()
+        return msg
+
+
 class PropertyYMLError(CompilationError):
     def __init__(self, path: str, issue: str):
         self.path = path
diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py
index a0b1f34de37..f17ee1d8e79 100644
--- a/core/dbt/parser/manifest.py
+++ b/core/dbt/parser/manifest.py
@@ -1262,11 +1262,18 @@ def _check_resource_uniqueness(
     config: RuntimeConfig,
 ) -> None:
     alias_resources: Dict[str, ManifestNode] = {}
+    versioned_resources: Dict[str, ManifestNode] = {}
+    unversioned_resources: Dict[str, ManifestNode] = {}
 
     for resource, node in manifest.nodes.items():
         if not node.is_relational:
             continue
 
+        if node.is_versioned:
+            versioned_resources[node.name] = node
+        else:
+            unversioned_resources[node.name] = node
+
         # the full node name is really defined by the adapter's relation
         relation_cls = get_relation_class_by_name(config.credentials.type)
         relation = relation_cls.create_from(config=config, node=node)
@@ -1280,6 +1287,17 @@ def _check_resource_uniqueness(
 
         alias_resources[full_node_name] = node
 
+    intersection_versioned = set(versioned_resources.keys()).intersection(
+        set(unversioned_resources.keys())
+    )
+    if intersection_versioned:
+        for name in intersection_versioned:
+            versioned_node = versioned_resources[name]
+            unversioned_node = unversioned_resources[name]
+            raise dbt.exceptions.DuplicateVersionedUnversionedError(
+                versioned_node, unversioned_node
+            )
+
 
 def _warn_for_unused_resource_config_paths(manifest: Manifest, config: RuntimeConfig) -> None:
     resource_fqns: Mapping[str, PathSet] = manifest.get_resource_fqns()
diff --git a/tests/functional/partial_parsing/test_partial_parsing.py b/tests/functional/partial_parsing/test_partial_parsing.py
index 76a06b69a3d..748747311c7 100644
--- a/tests/functional/partial_parsing/test_partial_parsing.py
+++ b/tests/functional/partial_parsing/test_partial_parsing.py
@@ -71,7 +71,7 @@
     groups_schema_yml_two_groups_private_orders_invalid_access,
 )
 
-from dbt.exceptions import CompilationError, ParsingError
+from dbt.exceptions import CompilationError, ParsingError, DuplicateVersionedUnversionedError
 from dbt.contracts.files import ParseFileType
 from dbt.contracts.results import TestStatus
 import re
@@ -358,6 +358,14 @@ def test_pp_versioned_models(self, project):
         write_file(model_two_sql, project.project_root, "models", "model_one_different.sql")
         results = run_dbt(["--partial-parse", "run"])
         assert len(results) == 3
+        manifest = get_manifest(project.project_root)
+        assert len(manifest.nodes) == 3
+        print(f"--- nodes: {manifest.nodes.keys()}")
+
+        # create a new model_one in model_one.sql and re-parse
+        write_file(model_one_sql, project.project_root, "models", "model_one.sql")
+        with pytest.raises(DuplicateVersionedUnversionedError):
+            run_dbt(["parse"])
 
 
 class TestSources:


Code review comments:
diff_hunk: @@ -1280,6 +1287,17 @@ def _check_resource_uniqueness(
 
         alias_resources[full_node_name] = node
 
+    intersection_versioned = set(versioned_resources.keys()).intersection(
+        set(unversioned_resources.keys())
+    )
+    if intersection_versioned:
path: core/dbt/parser/manifest.py
text: @user1:
I think raising this sets us back in terms of being able to handle ref-able resource with duplicate names across packages. For example - if I define project_a.model (unversioned) and project_b.model.v1 (versioned) - those work when run in their respective projects as root, but when installed as packages would raise `DuplicateVersionedUnversionedError`

@author:
I'm thinking that we need to store the versioned_resources and unversioned_resources by package. Maybe we'd only need to check the root package?

@author:
I've updated the code to check versioned/unversioned by packages and tweaked the error message to also suggest adding the unversioned model to the versions.

diff_hunk: @@ -2187,6 +2187,25 @@ def get_message(self) -> str:
         return msg
 
 
+class DuplicateVersionedUnversionedError(ParsingError):
+    def __init__(self, versioned_node, unversioned_node):
+        self.versioned_node = versioned_node
+        self.unversioned_node = unversioned_node
+        super().__init__(msg=self.get_message())
+
+    def get_message(self) -> str:
+        msg = f"""
+dbt found versioned and unversioned models with the name "{self.versioned_node.name}".
+
+Since these resources have the same name, dbt will be unable to find the correct resource
+when looking for ref({self.versioned_node.name}).
path: core/dbt/exceptions.py
text: @user1:
tiny nit - adding quotes around the node: `ref('{self.versioned_node.name}')`

@author:
Added quotes.