diff --git a/core/dbt/flags.py b/core/dbt/flags.py index 0d905598447..f1fccd40e07 100644 --- a/core/dbt/flags.py +++ b/core/dbt/flags.py @@ -1,5 +1,4 @@ STRICT_MODE = False -NON_DESTRUCTIVE = False FULL_REFRESH = False USE_CACHE = True WARN_ERROR = False @@ -7,11 +6,9 @@ def reset(): - global STRICT_MODE, NON_DESTRUCTIVE, FULL_REFRESH, USE_CACHE, WARN_ERROR, \ - TEST_NEW_PARSER + global STRICT_MODE, FULL_REFRESH, USE_CACHE, WARN_ERROR, TEST_NEW_PARSER STRICT_MODE = False - NON_DESTRUCTIVE = False FULL_REFRESH = False USE_CACHE = True WARN_ERROR = False diff --git a/core/dbt/include/global_project/macros/materializations/incremental/incremental.sql b/core/dbt/include/global_project/macros/materializations/incremental/incremental.sql index f53df57cd4e..0dd87ce10eb 100644 --- a/core/dbt/include/global_project/macros/materializations/incremental/incremental.sql +++ b/core/dbt/include/global_project/macros/materializations/incremental/incremental.sql @@ -23,21 +23,16 @@ schema=schema, database=database, type='table') -%} - {%- set non_destructive_mode = (flags.NON_DESTRUCTIVE == True) -%} {%- set full_refresh_mode = (flags.FULL_REFRESH == True) -%} {%- set exists_as_table = (old_relation is not none and old_relation.is_table) -%} {%- set exists_not_as_table = (old_relation is not none and not old_relation.is_table) -%} - {%- set should_truncate = (non_destructive_mode and full_refresh_mode and exists_as_table) -%} - {%- set should_drop = (not should_truncate and (full_refresh_mode or exists_not_as_table)) -%} - {%- set force_create = (flags.FULL_REFRESH and not flags.NON_DESTRUCTIVE) -%} + {%- set should_drop = (full_refresh_mode or exists_not_as_table) -%} -- setup {% if old_relation is none -%} -- noop - {%- elif should_truncate -%} - {{ adapter.truncate_relation(old_relation) }} {%- elif should_drop -%} {{ adapter.drop_relation(old_relation) }} {%- set old_relation = none -%} @@ -49,7 +44,7 @@ {{ run_hooks(pre_hooks, inside_transaction=True) }} -- build model - {% if force_create or old_relation is none -%} + {% if full_refresh_mode or old_relation is none -%} {%- call statement('main') -%} {{ create_table_as(False, target_relation, sql) }} {%- endcall -%} diff --git a/core/dbt/include/global_project/macros/materializations/table/table.sql b/core/dbt/include/global_project/macros/materializations/table/table.sql index d12062c50a9..fb50b76f684 100644 --- a/core/dbt/include/global_project/macros/materializations/table/table.sql +++ b/core/dbt/include/global_project/macros/materializations/table/table.sql @@ -2,7 +2,6 @@ {%- set identifier = model['alias'] -%} {%- set tmp_identifier = model['name'] + '__dbt_tmp' -%} {%- set backup_identifier = model['name'] + '__dbt_backup' -%} - {%- set non_destructive_mode = (flags.NON_DESTRUCTIVE == True) -%} {%- set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) -%} {%- set target_relation = api.Relation.create(identifier=identifier, @@ -24,23 +23,12 @@ {%- set exists_as_table = (old_relation is not none and old_relation.is_table) -%} {%- set exists_as_view = (old_relation is not none and old_relation.is_view) -%} - {%- set create_as_temporary = (exists_as_table and non_destructive_mode) -%} -- drop the temp relations if they exists for some reason {{ adapter.drop_relation(intermediate_relation) }} {{ adapter.drop_relation(backup_relation) }} - -- setup: if the target relation already exists, truncate or drop it (if it's a view) - {% if non_destructive_mode -%} - {% if exists_as_table -%} - {{ adapter.truncate_relation(old_relation) }} - {% elif exists_as_view -%} - {{ adapter.drop_relation(old_relation) }} - {%- set old_relation = none -%} - {%- endif %} - {%- endif %} - {{ run_hooks(pre_hooks, inside_transaction=False) }} -- `BEGIN` happens here: @@ -48,36 +36,15 @@ -- build model {% call statement('main') -%} - {%- if non_destructive_mode -%} - {%- if old_relation is not none -%} - {{ create_table_as(create_as_temporary, intermediate_relation, sql) }} - - {% set dest_columns = adapter.get_columns_in_relation(target_relation) %} - {% set dest_cols_csv = dest_columns | map(attribute='quoted') | join(', ') %} - - insert into {{ target_relation }} ({{ dest_cols_csv }}) ( - select {{ dest_cols_csv }} - from {{ intermediate_relation.include(database=(not create_as_temporary), - schema=(not create_as_temporary)) }} - ); - {%- else -%} - {{ create_table_as(create_as_temporary, target_relation, sql) }} - {%- endif -%} - {%- else -%} - {{ create_table_as(create_as_temporary, intermediate_relation, sql) }} - {%- endif -%} + {{ create_table_as(False, intermediate_relation, sql) }} {%- endcall %} -- cleanup - {% if non_destructive_mode -%} - -- noop - {%- else -%} - {% if old_relation is not none %} - {{ adapter.rename_relation(target_relation, backup_relation) }} - {% endif %} + {% if old_relation is not none %} + {{ adapter.rename_relation(target_relation, backup_relation) }} + {% endif %} - {{ adapter.rename_relation(intermediate_relation, target_relation) }} - {%- endif %} + {{ adapter.rename_relation(intermediate_relation, target_relation) }} {{ run_hooks(post_hooks, inside_transaction=True) }} diff --git a/core/dbt/include/global_project/macros/materializations/view/create_or_replace_view.sql b/core/dbt/include/global_project/macros/materializations/view/create_or_replace_view.sql index c9fb7095b44..62d4fbbf5c9 100644 --- a/core/dbt/include/global_project/macros/materializations/view/create_or_replace_view.sql +++ b/core/dbt/include/global_project/macros/materializations/view/create_or_replace_view.sql @@ -1,12 +1,10 @@ -{% macro handle_existing_table(full_refresh, non_destructive_mode, old_relation) %} - {{ adapter_macro("dbt.handle_existing_table", full_refresh, non_destructive_mode, old_relation) }} +{% macro handle_existing_table(full_refresh, old_relation) %} + {{ adapter_macro("dbt.handle_existing_table", full_refresh, old_relation) }} {% endmacro %} -{% macro default__handle_existing_table(full_refresh, non_destructive_mode, old_relation) %} - {%- if not non_destructive_mode -%} - {{ adapter.drop_relation(old_relation) }} - {%- endif -%} +{% macro default__handle_existing_table(full_refresh, old_relation) %} + {{ adapter.drop_relation(old_relation) }} {% endmacro %} {# /* @@ -23,7 +21,6 @@ {% macro create_or_replace_view(run_outside_transaction_hooks=True) %} {%- set identifier = model['alias'] -%} - {%- set non_destructive_mode = (flags.NON_DESTRUCTIVE == True) -%} {%- set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) -%} @@ -33,9 +30,6 @@ identifier=identifier, schema=schema, database=database, type='view') -%} - {%- set should_ignore = non_destructive_mode and exists_as_view %} - {%- set has_transactional_hooks = (hooks | selectattr('transaction', 'equalto', True) | list | length) > 0 %} - {% if run_outside_transaction_hooks %} -- no transactions on BigQuery {{ run_hooks(pre_hooks, inside_transaction=False) }} @@ -48,30 +42,17 @@ -- that's an error. If we were told to full refresh, drop it. This behavior differs -- for Snowflake and BigQuery, so multiple dispatch is used. {%- if old_relation is not none and old_relation.is_table -%} - {{ handle_existing_table(flags.FULL_REFRESH, non_destructive_mode, old_relation) }} + {{ handle_existing_table(flags.FULL_REFRESH, old_relation) }} {%- endif -%} -- build model - {% if non_destructive_mode -%} - {% call noop_statement('main', status="PASS", res=None) -%} - -- Not running : non-destructive mode - {{ sql }} - {%- endcall %} - {%- else -%} - {% call statement('main') -%} - {{ create_view_as(target_relation, sql) }} - {%- endcall %} - {%- endif %} + {% call statement('main') -%} + {{ create_view_as(target_relation, sql) }} + {%- endcall %} {{ run_hooks(post_hooks, inside_transaction=True) }} - {# - -- Don't commit in non-destructive mode _unless_ there are in-transaction hooks - -- TODO : Figure out some other way of doing this that isn't as fragile - #} - {% if has_transactional_hooks or not should_ignore %} - {{ adapter.commit() }} - {% endif %} + {{ adapter.commit() }} {% if run_outside_transaction_hooks %} -- No transactions on BigQuery diff --git a/core/dbt/include/global_project/macros/materializations/view/view.sql b/core/dbt/include/global_project/macros/materializations/view/view.sql index 2fa2a672678..58f533926b8 100644 --- a/core/dbt/include/global_project/macros/materializations/view/view.sql +++ b/core/dbt/include/global_project/macros/materializations/view/view.sql @@ -3,7 +3,6 @@ {%- set identifier = model['alias'] -%} {%- set tmp_identifier = model['name'] + '__dbt_tmp' -%} {%- set backup_identifier = model['name'] + '__dbt_backup' -%} - {%- set non_destructive_mode = (flags.NON_DESTRUCTIVE == True) -%} {%- set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) -%} {%- set target_relation = api.Relation.create(identifier=identifier, schema=schema, database=database, @@ -30,9 +29,6 @@ {%- set exists_as_view = (old_relation is not none and old_relation.is_view) -%} - {%- set has_transactional_hooks = (hooks | selectattr('transaction', 'equalto', True) | list | length) > 0 %} - {%- set should_ignore = non_destructive_mode and exists_as_view %} - {{ run_hooks(pre_hooks, inside_transaction=False) }} -- drop the temp relations if they exists for some reason @@ -43,45 +39,22 @@ {{ run_hooks(pre_hooks, inside_transaction=True) }} -- build model - {% if should_ignore -%} - {# - -- Materializations need to a statement with name='main'. - -- We could issue a no-op query here (like `select 1`), but that's wasteful. Instead: - -- 1) write the sql contents out to the compiled dirs - -- 2) return a status and result to the caller - #} - {% call noop_statement('main', status="PASS", res=None) -%} - -- Not running : non-destructive mode - {{ sql }} - {%- endcall %} - {%- else -%} - {% call statement('main') -%} - {{ create_view_as(intermediate_relation, sql) }} - {%- endcall %} - {%- endif %} + {% call statement('main') -%} + {{ create_view_as(intermediate_relation, sql) }} + {%- endcall %} -- cleanup - {% if not should_ignore -%} - -- 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) }} - {%- endif %} + -- 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) }} - {# - -- Don't commit in non-destructive mode _unless_ there are in-transaction hooks - -- TODO : Figure out some other way of doing this that isn't as fragile - #} - {% if has_transactional_hooks or not should_ignore %} - {{ adapter.commit() }} - {% endif %} + {{ adapter.commit() }} - {% if not should_ignore %} - {{ drop_relation_if_exists(backup_relation) }} - {% endif %} + {{ drop_relation_if_exists(backup_relation) }} {{ run_hooks(post_hooks, inside_transaction=False) }} diff --git a/core/dbt/main.py b/core/dbt/main.py index 9d96d9c91d9..d7a869be6ef 100644 --- a/core/dbt/main.py +++ b/core/dbt/main.py @@ -204,7 +204,6 @@ def run_from_args(parsed): def update_flags(parsed): - flags.NON_DESTRUCTIVE = getattr(parsed, 'non_destructive', False) flags.USE_CACHE = getattr(parsed, 'use_cache', True) arg_drop_existing = getattr(parsed, 'drop_existing', False) @@ -421,14 +420,6 @@ def _add_selection_arguments(*subparsers): def _add_table_mutability_arguments(*subparsers): for sub in subparsers: - sub.add_argument( - '--non-destructive', - action='store_true', - help=""" - If specified, DBT will not drop views. Tables will be truncated - instead of dropped. - """ - ) sub.add_argument( '--full-refresh', action='store_true', @@ -675,7 +666,7 @@ def parse_args(args): # --models, --exclude _add_selection_arguments(run_sub, compile_sub, generate_sub, test_sub, archive_sub) - # --full-refresh, --non-destructive + # --full-refresh _add_table_mutability_arguments(run_sub, compile_sub) _build_seed_subparser(subs, base_subparser) diff --git a/plugins/bigquery/dbt/include/bigquery/macros/materializations/incremental.sql b/plugins/bigquery/dbt/include/bigquery/macros/materializations/incremental.sql index 98c65a1dddc..fc84c11d91d 100644 --- a/plugins/bigquery/dbt/include/bigquery/macros/materializations/incremental.sql +++ b/plugins/bigquery/dbt/include/bigquery/macros/materializations/incremental.sql @@ -3,13 +3,8 @@ {%- set unique_key = config.get('unique_key') -%} - {%- set non_destructive_mode = (flags.NON_DESTRUCTIVE == True) -%} {%- set full_refresh_mode = (flags.FULL_REFRESH == True) -%} - {% if non_destructive_mode %} - {{ exceptions.raise_compiler_error("--non-destructive mode is not supported on BigQuery") }} - {% endif %} - {%- set identifier = model['alias'] -%} {%- set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) -%} diff --git a/plugins/bigquery/dbt/include/bigquery/macros/materializations/table.sql b/plugins/bigquery/dbt/include/bigquery/macros/materializations/table.sql index 1fb13779b2f..378b1c2ecea 100644 --- a/plugins/bigquery/dbt/include/bigquery/macros/materializations/table.sql +++ b/plugins/bigquery/dbt/include/bigquery/macros/materializations/table.sql @@ -29,7 +29,6 @@ {% materialization table, adapter='bigquery' -%} {%- set identifier = model['alias'] -%} - {%- set non_destructive_mode = (flags.NON_DESTRUCTIVE == True) -%} {%- set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) -%} {%- set exists_not_as_table = (old_relation is not none and not old_relation.is_table) -%} {%- set target_relation = api.Relation.create(database=database, schema=schema, identifier=identifier, type='table') -%} diff --git a/plugins/bigquery/dbt/include/bigquery/macros/materializations/view.sql b/plugins/bigquery/dbt/include/bigquery/macros/materializations/view.sql index 561c38bb5c3..7821f3e121b 100644 --- a/plugins/bigquery/dbt/include/bigquery/macros/materializations/view.sql +++ b/plugins/bigquery/dbt/include/bigquery/macros/materializations/view.sql @@ -1,6 +1,6 @@ -{% macro bigquery__handle_existing_table(full_refresh, non_destructive_mode, old_relation) %} - {%- if full_refresh and not non_destructive_mode -%} +{% macro bigquery__handle_existing_table(full_refresh, old_relation) %} + {%- if full_refresh -%} {{ adapter.drop_relation(old_relation) }} {%- else -%} {{ exceptions.relation_wrong_type(old_relation, 'view') }} diff --git a/plugins/snowflake/dbt/include/snowflake/macros/materializations/table.sql b/plugins/snowflake/dbt/include/snowflake/macros/materializations/table.sql index 7219d0aa0ac..2c5e4e2ea4e 100644 --- a/plugins/snowflake/dbt/include/snowflake/macros/materializations/table.sql +++ b/plugins/snowflake/dbt/include/snowflake/macros/materializations/table.sql @@ -2,7 +2,6 @@ {%- set identifier = model['alias'] -%} {%- set tmp_identifier = model['name'] + '__dbt_tmp' -%} {%- set backup_identifier = model['name'] + '__dbt_backup' -%} - {%- set non_destructive_mode = (flags.NON_DESTRUCTIVE == True) -%} {%- set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) -%} {%- set target_relation = api.Relation.create(identifier=identifier, @@ -28,22 +27,10 @@ {%- set exists_as_table = (old_relation is not none and old_relation.is_table) -%} {%- set exists_as_view = (old_relation is not none and old_relation.is_view) -%} - {%- set create_as_temporary = (exists_as_table and non_destructive_mode) -%} - -- drop the temp relations if they exists for some reason {{ adapter.drop_relation(intermediate_relation) }} - -- setup: if the target relation already exists, truncate or drop it (if it's a view) - {% if non_destructive_mode -%} - {% if exists_as_table -%} - {{ adapter.truncate_relation(old_relation) }} - {% elif exists_as_view -%} - {{ adapter.drop_relation(old_relation) }} - {%- set old_relation = none -%} - {%- endif %} - {%- endif %} - {{ run_hooks(pre_hooks, inside_transaction=False) }} -- `BEGIN` happens here: @@ -51,42 +38,22 @@ -- build model {% call statement('main') -%} - {%- if non_destructive_mode -%} - {%- if old_relation is not none -%} - {{ create_table_as(create_as_temporary, intermediate_relation, sql) }} - - {% set dest_columns = adapter.get_columns_in_relation(old_relation) %} - {% set dest_cols_csv = dest_columns | map(attribute='quoted') | join(', ') %} - - insert into {{ target_relation }} ({{ dest_cols_csv }}) ( - select {{ dest_cols_csv }} - from {{ intermediate_relation.include(database=(not create_as_temporary), schema=(not create_as_temporary)) }} - ); - {%- else -%} - {{ create_table_as(create_as_temporary, target_relation, sql) }} - {%- endif -%} - {%- else -%} - {{ create_table_as(create_as_temporary, intermediate_relation, sql) }} - {%- endif -%} + {{ create_table_as(False, intermediate_relation, sql) }} {%- endcall %} -- cleanup - {% if non_destructive_mode -%} - -- noop - {%- else -%} - {% if old_relation is not none %} - {% if old_relation.type == 'view' %} - {#-- This is the primary difference between Snowflake and Redshift. Renaming this view - -- would cause an error if the view has become invalid due to upstream schema changes #} - {{ log("Dropping relation " ~ old_relation ~ " because it is a view and this model is a table.") }} - {{ drop_relation_if_exists(old_relation) }} - {% else %} - {{ adapter.rename_relation(target_relation, backup_relation) }} - {% endif %} + {% if old_relation is not none %} + {% if old_relation.type == 'view' %} + {#-- This is the primary difference between Snowflake and Redshift. Renaming this view + -- would cause an error if the view has become invalid due to upstream schema changes #} + {{ log("Dropping relation " ~ old_relation ~ " because it is a view and this model is a table.") }} + {{ drop_relation_if_exists(old_relation) }} + {% else %} + {{ adapter.rename_relation(target_relation, backup_relation) }} {% endif %} + {% endif %} - {{ adapter.rename_relation(intermediate_relation, target_relation) }} - {%- endif %} + {{ adapter.rename_relation(intermediate_relation, target_relation) }} {{ run_hooks(post_hooks, inside_transaction=True) }} diff --git a/test/integration/017_runtime_materialization_tests/test_runtime_materialization.py b/test/integration/017_runtime_materialization_tests/test_runtime_materialization.py index 977911d2d4c..2236fe82fa0 100644 --- a/test/integration/017_runtime_materialization_tests/test_runtime_materialization.py +++ b/test/integration/017_runtime_materialization_tests/test_runtime_materialization.py @@ -43,47 +43,6 @@ def test_postgres_full_refresh(self): self.assertTablesEqual("seed","incremental") self.assertTablesEqual("seed","materialized") - @use_profile('postgres') - def test_postgres_non_destructive(self): - results = self.run_dbt(['run', '--non-destructive']) - self.assertEqual(len(results), 3) - - self.assertTablesEqual("seed","view") - self.assertTablesEqual("seed","incremental") - self.assertTablesEqual("seed","materialized") - self.assertTableDoesNotExist('dependent_view') - - self.run_sql_file("test/integration/017_runtime_materialization_tests/update.sql") - - results = self.run_dbt(['run', '--non-destructive']) - self.assertEqual(len(results), 3) - - self.assertTableDoesExist('dependent_view') - self.assertTablesEqual("seed","view") - self.assertTablesEqual("seed","incremental") - self.assertTablesEqual("seed","materialized") - - @use_profile('postgres') - def test_postgres_full_refresh_and_non_destructive(self): - results = self.run_dbt(['run', '--full-refresh', '--non-destructive']) - self.assertEqual(len(results), 3) - - self.assertTablesEqual("seed","view") - self.assertTablesEqual("seed","incremental") - self.assertTablesEqual("seed","materialized") - self.assertTableDoesNotExist('dependent_view') - - self.run_sql_file("test/integration/017_runtime_materialization_tests/invalidate_incremental.sql") - self.run_sql_file("test/integration/017_runtime_materialization_tests/update.sql") - - results = self.run_dbt(['run', '--full-refresh', '--non-destructive']) - self.assertEqual(len(results), 3) - - self.assertTableDoesExist('dependent_view') - self.assertTablesEqual("seed","view") - self.assertTablesEqual("seed","incremental") - self.assertTablesEqual("seed","materialized") - @use_profile('postgres') def test_postgres_delete__dbt_tmp_relation(self): # This creates a __dbt_tmp view - make sure it doesn't interfere with the dbt run diff --git a/test/integration/017_runtime_materialization_tests/update.sql b/test/integration/017_runtime_materialization_tests/update.sql index 7329b71f63e..a4cd7b6c655 100644 --- a/test/integration/017_runtime_materialization_tests/update.sql +++ b/test/integration/017_runtime_materialization_tests/update.sql @@ -1,7 +1,5 @@ --- create a view on top of the models. Since they shouldn't be drop-cascaded, --- this view will still be around after the dbt run --non-destructive - +-- create a view on top of the models create view {schema}.dependent_view as ( select count(*) from {schema}.materialized diff --git a/test/integration/022_bigquery_test/test_simple_bigquery_view.py b/test/integration/022_bigquery_test/test_simple_bigquery_view.py index 4ecf951103d..3755bc5bb94 100644 --- a/test/integration/022_bigquery_test/test_simple_bigquery_view.py +++ b/test/integration/022_bigquery_test/test_simple_bigquery_view.py @@ -41,7 +41,6 @@ def assert_nondupes_pass(self): self.assertEqual(result.status, 0) - class TestSimpleBigQueryRun(TestBaseBigQueryRun): @use_profile('bigquery') @@ -53,15 +52,6 @@ def test__bigquery_simple_run(self): self.assertEqual(len(results), 5) self.assert_nondupes_pass() - @use_profile('bigquery') - def test__bigquery_exists_non_destructive(self): - self.run_dbt(['seed']) - # first run dbt. this should work - self.assertEqual(len(self.run_dbt()), 5) - # then run dbt with --non-destructive. The view should still exist. - self.run_dbt(['run', '--non-destructive']) - self.assert_nondupes_pass() - class TestUnderscoreBigQueryRun(TestBaseBigQueryRun): prefix = "_test{}{:04}".format(int(time.time()), random.randint(0, 9999))