Tidying up materialization logic: relation names, cache loading, etc (#…

…5221)

* Clean up existing_relation, load_relation,  etc

* Add changelog entry

* Empty commit, retrigger CI

.changes/unreleased/Under the Hood-20220509-092628.yaml

@@ -0,0 +1,8 @@
+kind: Under the Hood
+body: 'Clean up materialization logic: more consistent relation names, loading from
+ cache'
+time: 2022-05-09T09:26:28.551068+02:00
+custom:
+ Author: jtcohen6
+ Issue: "2869"
+ PR: "4921"

core/dbt/include/global_project/macros/adapters/relation.sql

@@ -86,17 +86,21 @@
 {% endmacro %}
 
 
-{# a user-friendly interface into adapter.get_relation #}
-{% macro load_relation(relation) %}
+-- a user-friendly interface into adapter.get_relation
+{% macro load_cached_relation(relation) %}
  {% do return(adapter.get_relation(
  database=relation.database,
  schema=relation.schema,
  identifier=relation.identifier
  )) -%}
 {% endmacro %}
 
+-- old name for backwards compatibility
+{% macro load_relation(relation) %}
+ {{ return(load_cached_relation(relation)) }}
+{% endmacro %}
+
 
-{# not used much, here for backwards compatibility #}
 {% macro drop_relation_if_exists(relation) %}
  {% if relation is not none %}
  {{ adapter.drop_relation(relation) }}

core/dbt/include/global_project/macros/materializations/models/incremental/incremental.sql

@@ -1,26 +1,25 @@
 
 {% materialization incremental, default -%}
 
- {% set unique_key = config.get('unique_key') %}
- {% set target_relation = this.incorporate(type='table') %}
- {% set existing_relation = load_relation(this) %}
+ -- relations
+ {%- set existing_relation = load_cached_relation(this) -%}
+ {%- set target_relation = this.incorporate(type='table') -%}
  {%- set temp_relation = make_temp_relation(target_relation)-%}
- {%- set intermediate_relation = make_intermediate_relation(target_relation) -%}
- {%- set backup_identifier = make_backup_relation(target_relation, backup_relation_type=none) -%}
- {%- set full_refresh_mode = (should_full_refresh()) -%}
+ {%- set intermediate_relation = make_intermediate_relation(target_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) -%}
 
- {% set on_schema_change = incremental_validate_on_schema_change(config.get('on_schema_change'), default='ignore') %}
+ -- configs
+ {%- set unique_key = config.get('unique_key') -%}
+ {%- set full_refresh_mode = (should_full_refresh() or existing_relation.is_view) -%}
+ {%- set on_schema_change = incremental_validate_on_schema_change(config.get('on_schema_change'), default='ignore') -%}
 
  -- the temp_ and backup_ relations 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. This has to happen before
  -- BEGIN, in a separate transaction
- {% set preexisting_intermediate_relation = adapter.get_relation(identifier=intermediate_relation['identifier'],
- schema=schema,
- database=database) %}
- {% set preexisting_backup_relation = adapter.get_relation(identifier=backup_identifier['identifier'],
- schema=schema,
- database=database) %}
+ {%- set preexisting_intermediate_relation = load_cached_relation(intermediate_relation)-%}
+ {%- set preexisting_backup_relation = load_cached_relation(backup_relation) -%}
  {{ drop_relation_if_exists(preexisting_intermediate_relation) }}
  {{ drop_relation_if_exists(preexisting_backup_relation) }}
 
@@ -31,21 +30,13 @@
 
  {% set to_drop = [] %}
 
- {# -- first check whether we want to full refresh for source view or config reasons #}
- {% set trigger_full_refresh = (full_refresh_mode or existing_relation.is_view) %}
-
  {% if existing_relation is none %}
- {% set build_sql = create_table_as(False, target_relation, sql) %}
- {% elif trigger_full_refresh %}
- {#-- Make sure the backup doesn't exist so we don't encounter issues with the rename below #}
- {% set intermediate_relation = existing_relation.incorporate(path={"identifier": temp_relation['identifier']}) %}
- {% set backup_relation = existing_relation.incorporate(path={"identifier": backup_identifier['identifier']}) %}
-
- {% set build_sql = create_table_as(False, intermediate_relation, sql) %}
+ {% set build_sql = get_create_table_as_sql(False, target_relation, sql) %}
+ {% elif full_refresh_mode %}
+ {% set build_sql = get_create_table_as_sql(False, intermediate_relation, sql) %}
  {% set need_swap = true %}
- {% do to_drop.append(backup_relation) %}
  {% else %}
- {% do run_query(create_table_as(True, temp_relation, sql)) %}
+ {% do run_query(get_create_table_as_sql(True, temp_relation, sql)) %}
  {% do adapter.expand_target_column_types(
  from_relation=temp_relation,
  to_relation=target_relation) %}
@@ -65,6 +56,7 @@
  {% if need_swap %}
  {% do adapter.rename_relation(target_relation, backup_relation) %}
  {% do adapter.rename_relation(intermediate_relation, target_relation) %}
+ {% do to_drop.append(backup_relation) %}
  {% endif %}
 
  {% do persist_docs(target_relation, model) %}

core/dbt/include/global_project/macros/materializations/models/table/table.sql

@@ -1,27 +1,19 @@
 {% materialization table, default %}
- {%- set identifier = model['alias'] -%}
 
- {%- 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='table') -%}
+ {%- 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 = adapter.get_relation(identifier=intermediate_relation['identifier'],
- schema=schema,
- database=database) -%}
+ {%- 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 old_relation is none else old_relation.type -%}
+ {%- 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 = adapter.get_relation(identifier=backup_relation['identifier'],
- schema=schema,
- database=database) -%}
+ {%- set preexisting_backup_relation = load_cached_relation(backup_relation) -%}
 
  -- drop the temp relations if they exist already in the database
  {{ drop_relation_if_exists(preexisting_intermediate_relation) }}
@@ -38,8 +30,8 @@
  {%- endcall %}
 
  -- cleanup
- {% if old_relation is not none %}
- {{ adapter.rename_relation(old_relation, backup_relation) }}
+ {% if existing_relation is not none %}
+ {{ adapter.rename_relation(existing_relation, backup_relation) }}
  {% endif %}
 
  {{ adapter.rename_relation(intermediate_relation, target_relation) }}

core/dbt/include/global_project/macros/materializations/models/view/view.sql

@@ -1,37 +1,30 @@
 {%- materialization view, default -%}
 
- {%- set identifier = model['alias'] -%}
-
- {%- 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 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 = adapter.get_relation(identifier=intermediate_relation['identifier'],
- schema=schema,
- database=database) -%}
+ {%- 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 "old_relation" to this identifier,
+ 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 old_relation, if it exists,
- or else "view" as a sane default if it does not. Note that if the old_relation does not
+ 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 old_relation is none else old_relation.type -%}
+ {%- 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 = adapter.get_relation(identifier=backup_relation['identifier'],
- schema=schema,
- database=database) -%}
+ {%- set preexisting_backup_relation = load_cached_relation(backup_relation) -%}
 
  {{ run_hooks(pre_hooks, inside_transaction=False) }}
 
@@ -44,13 +37,13 @@
 
  -- build model
  {% call statement('main') -%}
- {{ create_view_as(intermediate_relation, sql) }}
+ {{ get_create_view_as_sql(intermediate_relation, sql) }}
  {%- endcall %}
 
  -- cleanup
  -- move the existing view out of the way
- {% if old_relation is not none %}
- {{ adapter.rename_relation(old_relation, backup_relation) }}
+ {% if existing_relation is not none %}
+ {{ adapter.rename_relation(existing_relation, backup_relation) }}
  {% endif %}
  {{ adapter.rename_relation(intermediate_relation, target_relation) }}
 

test/integration/017_runtime_materialization_tests/create_incremental__dbt_tmp.sql

@@ -0,0 +1,4 @@
+
+create table {schema}.incremental__dbt_tmp as ( 
+ select 1 as id 
+);