removed deprication for materialization-return and replaced it with a…

CHANGELOG.md

@@ -1,3 +1,13 @@
+## dbt 1.0.0 (Release TBD)
+
+### Features
+
+### Fixes
+
+### Under the hood
+
+- Enact deprecation for `materialization-return` and replace deprecation warning with an exception. ([#3896](https://github.com/dbt-labs/dbt/issues/3896))
+
 ## dbt 0.21.0 (Release TBD)
 
 ## dbt 0.21.0rc1 (September 20, 2021)

core/dbt/deprecations.py

@@ -57,22 +57,6 @@ class DispatchPackagesDeprecation(DBTDeprecation):
     '''
 
 
-class MaterializationReturnDeprecation(DBTDeprecation):
-    _name = 'materialization-return'
-
-    _description = '''\
-    The materialization ("{materialization}") did not explicitly return a list
-    of relations to add to the cache. By default the target relation will be
-    added, but this behavior will be removed in a future version of dbt.
-
-
-
-    For more information, see:
-
-    https://docs.getdbt.com/v0.15/docs/creating-new-materializations#section-6-returning-relations
-    '''
-
-
 class NotADictionaryDeprecation(DBTDeprecation):
     _name = 'not-a-dictionary'
 
@@ -178,7 +162,6 @@ def warn(name, *args, **kwargs):
 
 deprecations_list: List[DBTDeprecation] = [
     DispatchPackagesDeprecation(),
-    MaterializationReturnDeprecation(),
     NotADictionaryDeprecation(),
     ColumnQuotingDeprecation(),
     ModelsKeyNonModelDeprecation(),

core/dbt/task/run.py

@@ -16,7 +16,6 @@
     get_counts,
 )
 
-from dbt import deprecations
 from dbt import tracking
 from dbt import utils
 from dbt.adapters.base import BaseRelation
@@ -209,11 +208,12 @@ def _materialization_relations(
         self, result: Any, model
     ) -> List[BaseRelation]:
         if isinstance(result, str):
-            deprecations.warn('materialization-return',
-                              materialization=model.get_materialization())
-            return [
-                self.adapter.Relation.create_from(self.config, model)
-            ]
+            msg = (
+                'The materialization ("{}") did not explicitly return a '
+                'list of relations to add to the cache.'
+                .format(str(model.get_materialization()))
+            )
+            raise CompilationException(msg, node=model)
 
         if isinstance(result, dict):
             return _validate_materialization_relations_dict(result, model)

test/integration/012_deprecation_tests/test_deprecations.py

@@ -35,31 +35,6 @@ def test_postgres_deprecations(self):
         self.assertEqual(expected, deprecations.active_deprecations)
 
 
-class TestMaterializationReturnDeprecation(BaseTestDeprecations):
-    @property
-    def models(self):
-        return self.dir('custom-models')
-
-    @property
-    def project_config(self):
-        return {
-            'config-version': 2,
-            'macro-paths': [self.dir('custom-materialization-macros')],
-        }
-
-    @use_profile('postgres')
-    def test_postgres_deprecations_fail(self):
-        # this should fail at runtime
-        self.run_dbt(['--warn-error', 'run'], expect_pass=False)
-
-    @use_profile('postgres')
-    def test_postgres_deprecations(self):
-        self.assertEqual(deprecations.active_deprecations, set())
-        self.run_dbt()
-        expected = {'materialization-return'}
-        self.assertEqual(expected, deprecations.active_deprecations)
-
-
 class TestAdapterMacroDeprecation(BaseTestDeprecations):
     @property
     def models(self):
@@ -219,4 +194,4 @@ def test_postgres_package_redirect_fail(self):
             self.run_dbt(['--warn-error', 'deps'])
         exc_str = ' '.join(str(exc.exception).split())  # flatten all whitespace
         expected = "The `fishtown-analytics/dbt_utils` package is deprecated in favor of `dbt-labs/dbt_utils`"
-        assert expected in exc_str
+        assert expected in exc_str

test/integration/053_custom_materialization/override-view-return-no-relation/dbt_project.yml

@@ -0,0 +1,4 @@
+name: view_adapter_override
+version: 2
+macro-paths: ['macros']
+config-version: 2

test/integration/053_custom_materialization/override-view-return-no-relation/macros/override_view.sql

@@ -0,0 +1,66 @@
+{# copy+pasting the default view impl #}
+{% materialization view, default %}
+
+  {%- set identifier = model['alias'] -%}
+  {%- set tmp_identifier = model['name'] + '__dbt_tmp' -%}
+  {%- set backup_identifier = model['name'] + '__dbt_backup' -%}
+
+  {%- set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) -%}
+  {%- set target_relation = api.Relation.create(identifier=identifier, schema=schema, database=database,
+                                                type='view') -%}
+  {%- set intermediate_relation = api.Relation.create(identifier=tmp_identifier,
+                                                      schema=schema, database=database, type='view') -%}
+
+  /*
+     This relation (probably) doesn't exist yet. If it does exist, it's a leftover from
+     a previous run, and we're going to try to drop it immediately. At the end of this
+     materialization, we're going to rename the "old_relation" to this identifier,
+     and then we're going to drop it. In order to make sure we run the correct one of:
+       - drop view ...
+       - drop table ...
+
+     We need to set the type of this relation to be the type of the old_relation, if it exists,
+     or else "view" as a sane default if it does not. Note that if the old_relation does not
+     exist, then there is nothing to move out of the way and subsequentally drop. In that case,
+     this relation will be effectively unused.
+  */
+  {%- set backup_relation_type = 'view' if old_relation is none else old_relation.type -%}
+  {%- set backup_relation = api.Relation.create(identifier=backup_identifier,
+                                                schema=schema, database=database,
+                                                type=backup_relation_type) -%}
+
+  {%- set exists_as_view = (old_relation is not none and old_relation.is_view) -%}
+
+  {{ run_hooks(pre_hooks, inside_transaction=False) }}
+
+  -- drop the temp relations if they exists for some reason
+  {{ adapter.drop_relation(intermediate_relation) }}
+  {{ adapter.drop_relation(backup_relation) }}
+
+  -- `BEGIN` happens here:
+  {{ run_hooks(pre_hooks, inside_transaction=True) }}
+
+  -- build model
+  {% call statement('main') -%}
+    {{ create_view_as(intermediate_relation, sql) }}
+  {%- endcall %}
+
+  -- cleanup
+  -- move the existing view out of the way
+  {% if old_relation is not none %}
+    {{ adapter.rename_relation(target_relation, backup_relation) }}
+  {% endif %}
+  {{ adapter.rename_relation(intermediate_relation, target_relation) }}
+
+  {{ run_hooks(post_hooks, inside_transaction=True) }}
+
+  {{ adapter.commit() }}
+
+  {{ drop_relation_if_exists(backup_relation) }}
+
+  {{ run_hooks(post_hooks, inside_transaction=False) }}
+
+  {# do not return anything! #}
+  {# {{ return({'relations': [target_relation]}) }} #}
+
+{%- endmaterialization -%}

test/integration/053_custom_materialization/test_custom_materialization.py

@@ -98,3 +98,18 @@ def test_postgres_default_dependency(self):
         self.run_dbt(['deps'])
         # this should error because the override is buggy
         self.run_dbt(['run'], expect_pass=False)
+
+
+class TestOverrideDefaultReturn(BaseTestCustomMaterialization):
+    @property
+    def project_config(self):
+        return {
+            'config-version': 2,
+            'macro-paths': ['override-view-return-no-relation']
+        }
+
+    @use_profile('postgres')
+    def test_postgres_default_dependency(self):
+        self.run_dbt(['deps'])
+        results = self.run_dbt(['run'], expect_pass=False)
+        assert 'did not explicitly return a list of relations' in results[0].message