Raise CompilationException on duplicate model (#568)

* Raise CompilationException on duplicate model

Extend tests

* Ignore disabled models in parse_sql_nodes

Extend tests for duplicate model

* Fix preexisting models

* Use double quotes consistently

Rename model-1 to model-disabled

* Fix unit tests

* Raise exception on duplicate model across packages

Extend tests

dbt/loader.py

@@ -72,6 +72,30 @@ def load_project(cls, root_project, all_projects, project, project_name,
 
 class ModelLoader(ResourceLoader):
 
+    @classmethod
+    def load_all(cls, root_project, all_projects, macros=None):
+        to_return = {}
+
+        for project_name, project in all_projects.items():
+            project_loaded = cls.load_project(root_project,
+                                              all_projects,
+                                              project, project_name,
+                                              macros)
+
+            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
     def load_project(cls, root_project, all_projects, project, project_name,
                      macros):

dbt/parser.py

@@ -4,6 +4,7 @@
 import hashlib
 import collections
 
+import dbt.exceptions
 import dbt.flags
 import dbt.model
 import dbt.utils
@@ -272,14 +273,27 @@ def parse_sql_nodes(nodes, root_project, projects, tags=None, macros=None):
                              package_name,
                              node.get('name'))
 
-        # TODO if this is set, raise a compiler error
-        to_return[node_path] = parse_node(node,
-                                          node_path,
-                                          root_project,
-                                          projects.get(package_name),
-                                          projects,
-                                          tags=tags,
-                                          macros=macros)
+        node_parsed = parse_node(node,
+                                 node_path,
+                                 root_project,
+                                 projects.get(package_name),
+                                 projects,
+                                 tags=tags,
+                                 macros=macros)
+
+        # Ignore disabled nodes
+        if not node_parsed['config']['enabled']:
+            continue
+
+        # 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')))
+
+        to_return[node_path] = node_parsed
 
     dbt.contracts.graph.parsed.validate_nodes(to_return)
 

test/integration/008_schema_tests_test/models/schema.yml

@@ -53,10 +53,3 @@ table_failure_summary:
 
         relationships:
             - { from: favorite_color, to: ref('table_copy'), field: favorite_color }
-
-
-# this will fail if run, but we've disabled this model
-disabled:
-    constraints:
-        unique:
-            - not_unique

test/integration/011_invalid_model_tests/test_invalid_models.py

@@ -53,4 +53,4 @@ def test_view_with_incremental_attributes(self):
             # should throw
             self.assertTrue(False)
         except RuntimeError as e:
-            self.assertTrue("which is disabled" in str(e))
+            self.assertTrue("which was not found" in str(e))

test/integration/025_duplicate_model_test/models-1/model-enabled-1/model.sql

@@ -0,0 +1,8 @@
+{{
+    config(
+        enabled=True,
+        materialized="table",
+    )
+}}
+
+select 1

test/integration/025_duplicate_model_test/models-1/model-enabled-2/model.sql

@@ -0,0 +1,8 @@
+{{
+    config(
+        enabled=True,
+        materialized="table",
+    )
+}}
+
+select 1

test/integration/025_duplicate_model_test/models-2/model-disabled/model.sql

@@ -0,0 +1,8 @@
+{{
+    config(
+        enabled=False,
+        materialized="table",
+    )
+}}
+
+select 1

test/integration/025_duplicate_model_test/models-2/model-enabled/model.sql

@@ -0,0 +1,8 @@
+{{
+    config(
+        enabled=True,
+        materialized="table",
+    )
+}}
+
+select 1 as value

test/integration/025_duplicate_model_test/models-3/table.sql

@@ -0,0 +1,7 @@
+{{
+    config(
+        materialized = 'table',
+    )
+}}
+
+select 1 as item

test/integration/025_duplicate_model_test/models-4/table.sql

@@ -0,0 +1,8 @@
+{{
+    config(
+        materialized = 'table',
+        enabled = False,
+    )
+}}
+
+select 1 as item

test/integration/025_duplicate_model_test/test_duplicate_model.py

@@ -0,0 +1,155 @@
+from nose.plugins.attrib import attr
+
+from dbt.exceptions import CompilationException
+from test.integration.base import DBTIntegrationTest
+
+
+class TestDuplicateModelEnabled(DBTIntegrationTest):
+
+    def setUp(self):
+        DBTIntegrationTest.setUp(self)
+
+    @property
+    def schema(self):
+        return "duplicate_model_025"
+
+    @property
+    def models(self):
+        return "test/integration/025_duplicate_model_test/models-1"
+
+    @property
+    def profile_config(self):
+        return {
+            "test": {
+                "outputs": {
+                    "dev": {
+                        "type": "postgres",
+                        "threads": 1,
+                        "host": "database",
+                        "port": 5432,
+                        "user": "root",
+                        "pass": "password",
+                        "dbname": "dbt",
+                        "schema": self.unique_schema()
+                    },
+                },
+                "target": "dev"
+            }
+        }
+
+    @attr(type="postgres")
+    def test_duplicate_model_enabled(self):
+        message = "Found models with the same name:.*"
+        with self.assertRaisesRegexp(CompilationException, message):
+            self.run_dbt(["run"])
+
+
+class TestDuplicateModelDisabled(DBTIntegrationTest):
+
+    def setUp(self):
+        DBTIntegrationTest.setUp(self)
+
+    @property
+    def schema(self):
+        return "duplicate_model_025"
+
+    @property
+    def models(self):
+        return "test/integration/025_duplicate_model_test/models-2"
+
+    @property
+    def profile_config(self):
+        return {
+            "test": {
+                "outputs": {
+                    "dev": {
+                        "type": "postgres",
+                        "threads": 1,
+                        "host": "database",
+                        "port": 5432,
+                        "user": "root",
+                        "pass": "password",
+                        "dbname": "dbt",
+                        "schema": self.unique_schema()
+                    },
+                },
+                "target": "dev"
+            }
+        }
+
+    @attr(type="postgres")
+    def test_duplicate_model_disabled(self):
+        try:
+            self.run_dbt(["run"])
+        except CompilationException:
+            self.fail(
+                "Compilation Exception raised on disabled model")
+        query = "select value from {schema}.model" \
+                .format(schema=self.unique_schema())
+        result = self.run_sql(query, fetch="one")[0]
+        assert result == 1
+
+
+class TestDuplicateModelEnabledAcrossPackages(DBTIntegrationTest):
+
+    def setUp(self):
+        DBTIntegrationTest.setUp(self)
+
+    @property
+    def schema(self):
+        return "duplicate_model_025"
+
+    @property
+    def models(self):
+        return "test/integration/025_duplicate_model_test/models-3"
+
+    @property
+    def project_config(self):
+        return {
+            "repositories": [
+                'https://github.com/fishtown-analytics/dbt-integration-project@master'
+            ]
+        }
+
+    @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):
+            self.run_dbt(["run"])
+
+
+class TestDuplicateModelDisabledAcrossPackages(DBTIntegrationTest):
+
+    def setUp(self):
+        DBTIntegrationTest.setUp(self)
+        self.run_sql_file("test/integration/025_duplicate_model_test/seed.sql")
+
+    @property
+    def schema(self):
+        return "duplicate_model_025"
+
+    @property
+    def models(self):
+        return "test/integration/025_duplicate_model_test/models-4"
+
+    @property
+    def project_config(self):
+        return {
+            "repositories": [
+                'https://github.com/fishtown-analytics/dbt-integration-project@master'
+            ]
+        }
+
+    @attr(type="postgres")
+    def test_duplicate_model_disabled_across_packages(self):
+        self.run_dbt(["deps"])
+        try:
+            self.run_dbt(["run"])
+        except CompilationException:
+            self.fail(
+                "Compilation Exception raised on disabled model")
+        query = "select 1 from {schema}.table" \
+                .format(schema=self.unique_schema())
+        result = self.run_sql(query, fetch="one")[0]
+        assert result == 1

test/unit/test_graph.py

@@ -190,35 +190,6 @@ def test__model_materializations(self):
                                .get('materialized')
             self.assertEquals(actual, expected)
 
-    def test__model_enabled(self):
-        self.use_models({
-            'model_one': 'select * from events',
-            'model_two': "select * from {{ref('model_one')}}",
-        })
-
-        cfg = {
-            "models": {
-                "materialized": "table",
-                "test_models_compile": {
-                    "model_one": {"enabled": True},
-                    "model_two": {"enabled": False},
-                }
-            }
-        }
-
-        compiler = self.get_compiler(self.get_project(cfg))
-        graph, linker = compiler.compile()
-
-        six.assertCountEqual(
-            self, linker.nodes(),
-            ['model.test_models_compile.model_one',
-             'model.test_models_compile.model_two'])
-
-        six.assertCountEqual(
-            self, linker.edges(),
-            [('model.test_models_compile.model_one',
-              'model.test_models_compile.model_two',)])
-
     def test__model_incremental(self):
         self.use_models({
             'model_one': 'select * from events'

test/unit/test_parser.py

@@ -1101,48 +1101,6 @@ def test__other_project_config(self):
                     'raw_sql': self.find_input_by_name(
                         models, 'view').get('raw_sql')
                 },
-                'model.snowplow.disabled': {
-                    'name': 'disabled',
-                    'schema': 'analytics',
-                    'resource_type': 'model',
-                    'unique_id': 'model.snowplow.disabled',
-                    'fqn': ['snowplow', 'disabled'],
-                    'empty': False,
-                    'package_name': 'snowplow',
-                    'refs': [],
-                    'depends_on': {
-                        'nodes': [],
-                        'macros': []
-                    },
-                    'path': 'disabled.sql',
-                    'original_file_path': 'disabled.sql',
-                    'root_path': get_os_path('/usr/src/app'),
-                    'config': disabled_config,
-                    'tags': set(),
-                    'raw_sql': self.find_input_by_name(
-                        models, 'disabled').get('raw_sql')
-                },
-                'model.snowplow.package': {
-                    'name': 'package',
-                    'schema': 'analytics',
-                    'resource_type': 'model',
-                    'unique_id': 'model.snowplow.package',
-                    'fqn': ['snowplow', 'views', 'package'],
-                    'empty': False,
-                    'package_name': 'snowplow',
-                    'refs': [],
-                    'depends_on': {
-                        'nodes': [],
-                        'macros': []
-                    },
-                    'path': get_os_path('views/package.sql'),
-                    'original_file_path': get_os_path('views/package.sql'),
-                    'root_path': get_os_path('/usr/src/app'),
-                    'config': sort_config,
-                    'tags': set(),
-                    'raw_sql': self.find_input_by_name(
-                        models, 'package').get('raw_sql')
-                },
                 'model.snowplow.multi_sort': {
                     'name': 'multi_sort',
                     'schema': 'analytics',