From 382c52aca794349c47ec007d34cc95db3cec5dae Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Thu, 16 Sep 2021 11:32:49 -0500 Subject: [PATCH] removed deprication for materialization-return and replaced it with an exception --- core/dbt/deprecations.py | 17 ----- core/dbt/task/run.py | 12 ++-- .../test_deprecations.py | 25 ------- .../dbt_project.yml | 4 ++ .../macros/override_view.sql | 66 +++++++++++++++++++ .../test_custom_materialization.py | 16 +++++ 6 files changed, 92 insertions(+), 48 deletions(-) create mode 100644 test/integration/053_custom_materialization/override-view-return-no-relation/dbt_project.yml create mode 100644 test/integration/053_custom_materialization/override-view-return-no-relation/macros/override_view.sql diff --git a/core/dbt/deprecations.py b/core/dbt/deprecations.py index 9f3f2dc072a..b47819c6820 100644 --- a/core/dbt/deprecations.py +++ b/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(), diff --git a/core/dbt/task/run.py b/core/dbt/task/run.py index ffc5e93642d..7f93c931467 100644 --- a/core/dbt/task/run.py +++ b/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) diff --git a/test/integration/012_deprecation_tests/test_deprecations.py b/test/integration/012_deprecation_tests/test_deprecations.py index 25973dcff56..4828c67f30a 100644 --- a/test/integration/012_deprecation_tests/test_deprecations.py +++ b/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(strict=True, expect_pass=False) - - @use_profile('postgres') - def test_postgres_deprecations(self): - self.assertEqual(deprecations.active_deprecations, set()) - self.run_dbt(strict=False) - expected = {'materialization-return'} - self.assertEqual(expected, deprecations.active_deprecations) - - class TestAdapterMacroDeprecation(BaseTestDeprecations): @property def models(self): diff --git a/test/integration/053_custom_materialization/override-view-return-no-relation/dbt_project.yml b/test/integration/053_custom_materialization/override-view-return-no-relation/dbt_project.yml new file mode 100644 index 00000000000..96c7226d89d --- /dev/null +++ b/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 diff --git a/test/integration/053_custom_materialization/override-view-return-no-relation/macros/override_view.sql b/test/integration/053_custom_materialization/override-view-return-no-relation/macros/override_view.sql new file mode 100644 index 00000000000..ab0a0bfcd0f --- /dev/null +++ b/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 -%} \ No newline at end of file diff --git a/test/integration/053_custom_materialization/test_custom_materialization.py b/test/integration/053_custom_materialization/test_custom_materialization.py index 0d4e5399934..b29591da5cd 100644 --- a/test/integration/053_custom_materialization/test_custom_materialization.py +++ b/test/integration/053_custom_materialization/test_custom_materialization.py @@ -98,3 +98,19 @@ 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']) + self.run_dbt(['run'], expect_pass=False) + +