add relation macros and add relation renameable/replaceable config (#603

)

* add relation macros and add relation renameable/replaceable config

* formatting cleanup

* PR fixes

* make tables irreplaceable

.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

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.

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 %}

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 -%}

dbt/include/redshift/macros/relations/table/drop.sql

@@ -0,0 +1,3 @@
+{%- macro redshift__drop_table(relation) -%}
+    drop table if exists {{ relation }} cascade
+{%- endmacro -%}

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 %}

dbt/include/redshift/macros/relations/view/drop.sql

@@ -0,0 +1,3 @@
+{%- macro redshift__drop_view(relation) -%}
+    drop view if exists {{ relation }} cascade
+{%- endmacro -%}

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 %}

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 %}

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)