check for node name uniqueness across refable resource types (#737)

* check for node name uniqueness across refable resource types

* change test for new error msg

* consistent error message for dupes

* small refactor, improve error msg

* fix tests

dbt/compilation.py

@@ -261,6 +261,22 @@ def get_all_projects(self):
 
         return all_projects
 
+    def _check_resource_uniqueness(cls, flat_graph):
+        nodes = flat_graph['nodes']
+        names_resources = {}
+
+        for resource, node in nodes.items():
+            if node.get('resource_type') not in NodeType.refable():
+                continue
+
+            name = node['name']
+            existing_node = names_resources.get(name)
+            if existing_node is not None:
+                dbt.exceptions.raise_duplicate_resource_name(
+                        existing_node, node)
+
+            names_resources[name] = node
+
     def compile(self):
         linker = Linker()
 
@@ -270,6 +286,8 @@ def compile(self):
         flat_graph = dbt.loader.GraphLoader.load_all(
             root_project, all_projects)
 
+        self._check_resource_uniqueness(flat_graph)
+
         flat_graph = dbt.parser.process_refs(flat_graph,
                                              root_project.get('name'))
 

dbt/exceptions.py

@@ -291,3 +291,17 @@ def raise_dep_not_found(node, node_description, required_pkg):
         'Error while parsing {}.\nThe required package "{}" was not found. '
         'Is the package installed?\nHint: You may need to run '
         '`dbt deps`.'.format(node_description, required_pkg), node=node)
+
+
+def raise_duplicate_resource_name(node_1, node_2):
+    duped_name = node_1['name']
+
+    raise_compiler_error(
+        'dbt found two resources with the name "{}". Since these resources '
+        'have the same name,\ndbt will be unable to find the correct resource '
+        'when ref("{}") is used. To fix this,\nchange the name of one of '
+        'these resources:\n- {} ({})\n- {} ({})'.format(
+            duped_name,
+            duped_name,
+            node_1['unique_id'], node_1['original_file_path'],
+            node_2['unique_id'], node_2['original_file_path']))

dbt/loader.py

@@ -84,16 +84,6 @@ def load_all(cls, root_project, all_projects, macros=None):
 
             to_return.update(project_loaded)
 
-        # Check for duplicate model names
-        names_models = {}
-        for model, attribs in to_return.items():
-            name = attribs['name']
-            existing_name = names_models.get(name)
-            if existing_name is not None:
-                raise dbt.exceptions.CompilationException(
-                    'Found models with the same name: \n- %s\n- %s' % (
-                        model, existing_name))
-            names_models[name] = model
         return to_return
 
     @classmethod

dbt/node_runners.py

@@ -79,7 +79,7 @@ def raise_on_first_error(self):
 
     @classmethod
     def is_refable(cls, node):
-        return node.get('resource_type') in [NodeType.Model, NodeType.Seed]
+        return node.get('resource_type') in NodeType.refable()
 
     @classmethod
     def is_ephemeral(cls, node):

dbt/node_types.py

@@ -20,6 +20,13 @@ def executable(cls):
             cls.Seed,
         ]
 
+    @classmethod
+    def refable(cls):
+        return [
+            cls.Model,
+            cls.Seed,
+        ]
+
 
 class RunHookType:
     Start = 'on-run-start'

dbt/parser.py

@@ -288,10 +288,8 @@ def parse_sql_nodes(nodes, root_project, projects, tags=None, macros=None):
         # Check for duplicate model names
         existing_node = to_return.get(node_path)
         if existing_node is not None:
-            raise dbt.exceptions.CompilationException(
-                'Found models with the same name:\n- %s\n- %s' % (
-                    existing_node.get('original_file_path'),
-                    node.get('original_file_path')))
+            dbt.exceptions.raise_duplicate_resource_name(
+                    existing_node, node_parsed)
 
         to_return[node_path] = node_parsed
 

dbt/utils.py

@@ -133,7 +133,7 @@ def model_immediate_name(model, non_destructive):
 
 def find_refable_by_name(flat_graph, target_name, target_package):
     return find_by_name(flat_graph, target_name, target_package,
-                        'nodes', [NodeType.Model, NodeType.Seed])
+                        'nodes', NodeType.refable())
 
 
 def find_macro_by_name(flat_graph, target_name, target_package):

test/integration/025_duplicate_model_test/test_duplicate_model.py

@@ -39,9 +39,12 @@ def profile_config(self):
 
     @attr(type="postgres")
     def test_duplicate_model_enabled(self):
-        message = "Found models with the same name:.*"
-        with self.assertRaisesRegexp(CompilationException, message):
+        message = "dbt found two resources with the name"
+        try:
             self.run_dbt(["run"])
+            self.assertTrue(False, "dbt did not throw for duplicate models")
+        except CompilationException as e:
+            self.assertTrue(message in str(e), "dbt did not throw the correct error message")
 
 
 class TestDuplicateModelDisabled(DBTIntegrationTest):
@@ -114,9 +117,12 @@ def project_config(self):
     @attr(type="postgres")
     def test_duplicate_model_enabled_across_packages(self):
         self.run_dbt(["deps"])
-        message = "Found models with the same name:.*"
-        with self.assertRaisesRegexp(CompilationException, message):
+        message = "dbt found two resources with the name"
+        try:
             self.run_dbt(["run"])
+            self.assertTrue(False, "dbt did not throw for duplicate models")
+        except CompilationException as e:
+            self.assertTrue(message in str(e), "dbt did not throw the correct error message")
 
 
 class TestDuplicateModelDisabledAcrossPackages(DBTIntegrationTest):