diff --git a/.changes/unreleased/Fixes-20230912-133327.yaml b/.changes/unreleased/Fixes-20230912-133327.yaml index afa2f3657..14528cad6 100644 --- a/.changes/unreleased/Fixes-20230912-133327.yaml +++ b/.changes/unreleased/Fixes-20230912-133327.yaml @@ -1,5 +1,5 @@ -kind: Fixes -body: use get_replace_sql in redshift__get_alter_materialized_view_as_sql +kind: Features +body: use get_replace_sql in redshift__get_alter_materialized_view_as_sql, avoid renaming materialized views with custom table.sql and view.sql time: 2023-09-12T13:33:27.451042-07:00 custom: Author: colin-rogers-dbt diff --git a/dbt/adapters/redshift/relation.py b/dbt/adapters/redshift/relation.py index 0ef4fe276..182b9b810 100644 --- a/dbt/adapters/redshift/relation.py +++ b/dbt/adapters/redshift/relation.py @@ -32,6 +32,17 @@ class RedshiftRelation(BaseRelation): relation_configs = { RelationType.MaterializedView.value: RedshiftMaterializedViewConfig, } + renameable_relations = frozenset( + { + RelationType.View, + RelationType.Table, + } + ) + replaceable_relations = frozenset( + { + RelationType.View, + } + ) def __post_init__(self): # Check for length of Redshift table/view names. diff --git a/dbt/include/redshift/macros/materializations/table.sql b/dbt/include/redshift/macros/materializations/table.sql new file mode 100644 index 000000000..907c83874 --- /dev/null +++ b/dbt/include/redshift/macros/materializations/table.sql @@ -0,0 +1,69 @@ +{% materialization table, adapter='redshift' %} + + {%- set existing_relation = load_cached_relation(this) -%} + {%- set target_relation = this.incorporate(type='table') %} + {%- set intermediate_relation = make_intermediate_relation(target_relation) -%} + -- the intermediate_relation should not already exist in the database; get_relation + -- will return None in that case. Otherwise, we get a relation that we can drop + -- later, before we try to use this name for the current operation + {%- set preexisting_intermediate_relation = load_cached_relation(intermediate_relation) -%} + /* + See ../view/view.sql for more information about this relation. + */ + {%- set backup_relation_type = 'table' if existing_relation is none else existing_relation.type -%} + {%- set backup_relation = make_backup_relation(target_relation, backup_relation_type) -%} + -- as above, the backup_relation should not already exist + {%- set preexisting_backup_relation = load_cached_relation(backup_relation) -%} + -- grab current tables grants config for comparision later on + {% set grant_config = config.get('grants') %} + + -- drop the temp relations if they exist already in the database + {{ drop_relation_if_exists(preexisting_intermediate_relation) }} + {{ drop_relation_if_exists(preexisting_backup_relation) }} + + {{ run_hooks(pre_hooks, inside_transaction=False) }} + + -- `BEGIN` happens here: + {{ run_hooks(pre_hooks, inside_transaction=True) }} + + -- build model + {% call statement('main') -%} + {{ get_create_table_as_sql(False, intermediate_relation, sql) }} + {%- endcall %} + + -- cleanup + {% if existing_relation is not none %} + /* Do the equivalent of rename_if_exists. 'existing_relation' could have been dropped + since the variable was first set. */ + {% set existing_relation = load_cached_relation(existing_relation) %} + {% if existing_relation is not none %} + {% if existing_relation.can_be_renamed %} + {{ adapter.rename_relation(existing_relation, backup_relation) }} + {% else %} + {{ drop_relation_if_exists(existing_relation) }} + {% endif %} + {% endif %} + {% endif %} + + + {{ adapter.rename_relation(intermediate_relation, target_relation) }} + + {% do create_indexes(target_relation) %} + + {{ run_hooks(post_hooks, inside_transaction=True) }} + + {% set should_revoke = should_revoke(existing_relation, full_refresh_mode=True) %} + {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} + + {% do persist_docs(target_relation, model) %} + + -- `COMMIT` happens here + {{ adapter.commit() }} + + -- finally, drop the existing/backup relation after the commit + {{ drop_relation_if_exists(backup_relation) }} + + {{ run_hooks(post_hooks, inside_transaction=False) }} + + {{ return({'relations': [target_relation]}) }} +{% endmaterialization %} diff --git a/dbt/include/redshift/macros/materializations/view.sql b/dbt/include/redshift/macros/materializations/view.sql new file mode 100644 index 000000000..f353f913f --- /dev/null +++ b/dbt/include/redshift/macros/materializations/view.sql @@ -0,0 +1,77 @@ +{%- materialization view, adapter='redshift' -%} + + {%- set existing_relation = load_cached_relation(this) -%} + {%- set target_relation = this.incorporate(type='view') -%} + {%- set intermediate_relation = make_intermediate_relation(target_relation) -%} + + -- the intermediate_relation should not already exist in the database; get_relation + -- will return None in that case. Otherwise, we get a relation that we can drop + -- later, before we try to use this name for the current operation + {%- set preexisting_intermediate_relation = load_cached_relation(intermediate_relation) -%} + /* + 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 "existing_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 existing_relation, if it exists, + or else "view" as a sane default if it does not. Note that if the existing_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 existing_relation is none else existing_relation.type -%} + {%- set backup_relation = make_backup_relation(target_relation, backup_relation_type) -%} + -- as above, the backup_relation should not already exist + {%- set preexisting_backup_relation = load_cached_relation(backup_relation) -%} + -- grab current tables grants config for comparision later on + {% set grant_config = config.get('grants') %} + + {{ run_hooks(pre_hooks, inside_transaction=False) }} + + -- drop the temp relations if they exist already in the database + {{ drop_relation_if_exists(preexisting_intermediate_relation) }} + {{ drop_relation_if_exists(preexisting_backup_relation) }} + + -- `BEGIN` happens here: + {{ run_hooks(pre_hooks, inside_transaction=True) }} + + -- build model + {% call statement('main') -%} + {{ get_create_view_as_sql(intermediate_relation, sql) }} + {%- endcall %} + + -- cleanup + -- move the existing view out of the way + {% if existing_relation is not none %} + /* Do the equivalent of rename_if_exists. 'existing_relation' could have been dropped + since the variable was first set. */ + {% set existing_relation = load_cached_relation(existing_relation) %} + {% if existing_relation is not none %} + {% if existing_relation.can_be_renamed %} + {{ adapter.rename_relation(existing_relation, backup_relation) }} + {% else %} + {{ drop_relation_if_exists(existing_relation) }} + {% endif %} + {% endif %} + {% endif %} + + {{ adapter.rename_relation(intermediate_relation, target_relation) }} + + {% set should_revoke = should_revoke(existing_relation, full_refresh_mode=True) %} + {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} + + {% do persist_docs(target_relation, model) %} + + {{ run_hooks(post_hooks, inside_transaction=True) }} + + {{ adapter.commit() }} + + {{ drop_relation_if_exists(backup_relation) }} + + {{ run_hooks(post_hooks, inside_transaction=False) }} + + {{ return({'relations': [target_relation]}) }} + +{%- endmaterialization -%} diff --git a/dbt/include/redshift/macros/relations/drop.sql b/dbt/include/redshift/macros/relations/drop.sql deleted file mode 100644 index 2709f7763..000000000 --- a/dbt/include/redshift/macros/relations/drop.sql +++ /dev/null @@ -1,7 +0,0 @@ -{% macro redshift__get_drop_relation_sql(relation) %} - {%- if relation.is_materialized_view -%} - {{ redshift__drop_materialized_view(relation) }} - {%- else -%} - drop {{ relation.type }} if exists {{ relation }} cascade - {%- endif -%} -{% endmacro %} diff --git a/dbt/include/redshift/macros/relations/materialized_view/_replace.sql b/dbt/include/redshift/macros/relations/materialized_view/_replace.sql deleted file mode 100644 index 2146e0967..000000000 --- a/dbt/include/redshift/macros/relations/materialized_view/_replace.sql +++ /dev/null @@ -1,4 +0,0 @@ -{% macro redshift__get_replace_materialized_view_as_sql(relation, sql, existing_relation, backup_relation, intermediate_relation) %} - {{ redshift__get_drop_relation_sql(existing_relation) }}; - {{ get_create_materialized_view_as_sql(relation, sql) }} -{% endmacro %} diff --git a/dbt/include/redshift/macros/relations/table/drop.sql b/dbt/include/redshift/macros/relations/table/drop.sql new file mode 100644 index 000000000..64ffc1f22 --- /dev/null +++ b/dbt/include/redshift/macros/relations/table/drop.sql @@ -0,0 +1,3 @@ +{%- macro redshift__drop_table(relation) -%} + drop table if exists {{ relation }} cascade +{%- endmacro -%} diff --git a/dbt/include/redshift/macros/relations/table/rename.sql b/dbt/include/redshift/macros/relations/table/rename.sql new file mode 100644 index 000000000..08fd5a172 --- /dev/null +++ b/dbt/include/redshift/macros/relations/table/rename.sql @@ -0,0 +1,3 @@ +{% macro redshift__get_rename_table_sql(relation, new_name) %} + alter table {{ relation }} rename to {{ new_name }} +{% endmacro %} diff --git a/dbt/include/redshift/macros/relations/view/drop.sql b/dbt/include/redshift/macros/relations/view/drop.sql new file mode 100644 index 000000000..cba066a53 --- /dev/null +++ b/dbt/include/redshift/macros/relations/view/drop.sql @@ -0,0 +1,3 @@ +{%- macro redshift__drop_view(relation) -%} + drop view if exists {{ relation }} cascade +{%- endmacro -%} diff --git a/dbt/include/redshift/macros/relations/view/rename.sql b/dbt/include/redshift/macros/relations/view/rename.sql new file mode 100644 index 000000000..0c6cdcdfa --- /dev/null +++ b/dbt/include/redshift/macros/relations/view/rename.sql @@ -0,0 +1,3 @@ +{% macro redshift__get_rename_view_sql(relation, new_name) %} + alter view {{ relation }} rename to {{ new_name }} +{% endmacro %} diff --git a/dbt/include/redshift/macros/relations/view/replace.sql b/dbt/include/redshift/macros/relations/view/replace.sql new file mode 100644 index 000000000..25a9d8b38 --- /dev/null +++ b/dbt/include/redshift/macros/relations/view/replace.sql @@ -0,0 +1,18 @@ +{% macro redshift__get_replace_view_sql(relation, sql) -%} + + {%- set binding = config.get('bind', default=True) -%} + + {% set bind_qualifier = '' if binding else 'with no schema binding' %} + {%- set sql_header = config.get('sql_header', none) -%} + + {{ sql_header if sql_header is not none }} + + create or replace view {{ relation }} + {%- set contract_config = config.get('contract') -%} + {%- if contract_config.enforced -%} + {{ get_assert_columns_equivalent(sql) }} + {%- endif %} as ( + {{ sql }} + ) {{ bind_qualifier }}; + +{%- endmacro %} diff --git a/tests/functional/adapter/materialized_view_tests/test_materialized_views.py b/tests/functional/adapter/materialized_view_tests/test_materialized_views.py index be5414122..e61737036 100644 --- a/tests/functional/adapter/materialized_view_tests/test_materialized_views.py +++ b/tests/functional/adapter/materialized_view_tests/test_materialized_views.py @@ -70,18 +70,6 @@ def test_materialized_view_create_idempotent(self, project, my_materialized_view ) assert self.query_relation_type(project, my_materialized_view) == "materialized_view" - @pytest.mark.skip( - "The current implementation does not support overwriting materialized views with tables." - ) - def test_table_replaces_materialized_view(self, project, my_materialized_view): - super().test_table_replaces_materialized_view(project, my_materialized_view) - - @pytest.mark.skip( - "The current implementation does not support overwriting materialized views with views." - ) - def test_view_replaces_materialized_view(self, project, my_materialized_view): - super().test_view_replaces_materialized_view(project, my_materialized_view) - class RedshiftMaterializedViewChanges(MaterializedViewChanges): @pytest.fixture(scope="class", autouse=True)