Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ADAP-387: Stub materialized view as a materialization #7211

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9c40e87
changie
mikealfare Mar 22, 2023
19aec7b
changie
mikealfare Mar 22, 2023
cda0eb4
init attempt at mv and basic forms of helper macros by mixing view an…
McKnight-42 Mar 24, 2023
40379b3
init attempt at mv and basic forms of helper macros by mixing view an…
McKnight-42 Mar 24, 2023
37eb64f
remove unneeded return statement, rename directory
McKnight-42 Mar 27, 2023
65672a9
remove unneeded ()
McKnight-42 Mar 27, 2023
f794560
Merge branch 'feature/materialized-views/ADAP-2' of github.com:dbt-la…
McKnight-42 Mar 28, 2023
845d7a3
responding to some pr feedback
McKnight-42 Mar 29, 2023
2b9faaa
adjusting order of events for mv base work
McKnight-42 Mar 29, 2023
158b9f9
move up prexisting drop of backup
McKnight-42 Mar 29, 2023
3287562
change relatiion type to view to be consistent
McKnight-42 Mar 29, 2023
eca26b8
add base test case
McKnight-42 Mar 29, 2023
f3faee2
fix jinja exeception message expression, basic test passing
McKnight-42 Mar 30, 2023
e4c7005
Merge branch 'feature/materialized-views/ADAP-2' of github.com:dbt-la…
McKnight-42 Mar 30, 2023
53b4adc
response to feedback, removeal of refresh infavor of combined create_…
McKnight-42 Mar 31, 2023
88f6cdd
swapping to api layer and stratgeies for default implementation (basi…
McKnight-42 Mar 31, 2023
056a356
remove stratgey to limit need for now
McKnight-42 Mar 31, 2023
66754dc
remove unneeded story level changelog entry
McKnight-42 Mar 31, 2023
0252ea3
add strategies to condtional in place of old macros
McKnight-42 Mar 31, 2023
fcc480d
macro name fix
McKnight-42 Mar 31, 2023
345c2b2
rename refresh macro in api level
McKnight-42 Mar 31, 2023
caad12c
align names between postgres and default to same convention
McKnight-42 Mar 31, 2023
d6eee2d
align names between postgres and default to same convention
McKnight-42 Mar 31, 2023
60a5392
change a create call to full refresh
McKnight-42 Mar 31, 2023
e9c423e
pull adapter rename into strategy, add backup_relation as optional arg
McKnight-42 Apr 3, 2023
eeb0d3f
minor typo fix, add intermediate relation to refresh strategy and ini…
McKnight-42 Apr 4, 2023
33a025b
Merge branch 'feature/materialized-views/ADAP-2' of github.com:dbt-la…
McKnight-42 Apr 4, 2023
703cff4
updating to feature main
McKnight-42 Apr 4, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{% macro db_api__materialized_view__create(relation, sql) %}
{{ adapter.dispatch('db_api__materialized_view__create', 'dbt')(relation, sql) }}
{% endmacro %}

{% macro default__db_api__materialized_view__create(relation, sql) -%}

{{ exceptions.raise_not_implemented(
'db_api__materialized_view__create macro not implemented for adapter '+adapter.type()) }}

{% endmacro %}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{% macro db_api__materialized_view__refresh(relation) %}
{{ adapter.dispatch('db_api__materialized_view__refresh', 'dbt')(relation) }}
{% endmacro %}

{% macro default__db_api__materialized_view__refresh(relation) -%}

{{ exceptions.raise_not_implemented(
'db_api__materialized_view__refresh not implemented for adapter '+adapter.type()) }}

{% endmacro %}
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
{%- materialization materialized_view, default -%}
{% set full_refresh_mode = should_full_refresh() %}
{%- 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) }}

-- cleanup
-- move the existing view out of the way
{% if existing_relation is none %}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referencing @Fleid's direction here for convenience:

For the full refresh, I think we should try to, in order:

  • CREATE OR REPLACE - if CREATE OR REPLACE is supported
  • CREATE, DROP, ALTER NAME - if rename is supported
  • DROP, CREATE - if none of the above

For on_configuration_changes, we should try to, in order:

  • ALTER in place
  • Apply the full refresh order

This is going to vary by adapter; but I'd like to avoid creating versions of this materialization in adapters if possible. Hence I think we should adopt something like the strategies approach that we did with the incremental materialization. In other words, we have "replace" strategy, an "alter" strategy, a "refresh" strategy, etc. In some cases, those strategies may point to the same macros (e.g. create_materialized_view_as()); but that would allow maintainers to update just the piece that is specific to that platform without needing to maintain the rest of the materialization.

{% set build_sql = strategy__materialized_view__create(target_relation, sql) %}
{% elif full_refresh_mode or existing_relation.type != 'view' %}
McKnight-42 marked this conversation as resolved.
Show resolved Hide resolved
{% set build_sql = strategy__materialized_view__full_refresh(target_relation, sql, backup_relation, intermediate_relation) %}
{% else %}
{% set build_sql = strategy__materialized_view__full_refresh(target_relation) %}
{% endif %}
mikealfare marked this conversation as resolved.
Show resolved Hide resolved

-- possible conditional wrapper for build_sql statement based on configs as a option?
-- build model
{% call statement("main") %}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to make sure this can actually exist outside of the if block above. For example, does adapter.rename() occur when you call it? Or does it just return the sql to do so? We're fine if it's the latter. But if it's the former, we'll do all the thing that happen immediately during the if block, and then do all the things that we return sql for during this phase. That may not be the right order.

{{ build_sql }}
{% endcall %}

{% 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) }}
McKnight-42 marked this conversation as resolved.
Show resolved Hide resolved
{{ drop_relation_if_exists(intermediate_relation) }}

{{ run_hooks(post_hooks, inside_transaction=False) }}

{{ return({'relations': [target_relation]}) }}

{%- endmaterialization -%}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{% macro strategy__materialized_view__create(relation, sql) %}
McKnight-42 marked this conversation as resolved.
Show resolved Hide resolved
{{ adapter.dispatch('strategy__materialized_view__create', 'dbt')(relation, sql) }}
{% endmacro %}

{% macro default__strategy__materialized_view__create(relation, sql) %}
{{ db_api__materialized_view__create(relation, sql) }}
{% endmacro %}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{% macro strategy__materialized_view__full_refresh(relation, sql, backup_relation=None, intermediate_relation=None) %}
{{ adapter.dispatch('strategy__materialized_view__full_refresh', 'dbt')(relation, sql, backup_relation, intermediate_relation) }}
{% endmacro %}


{% macro default__strategy__materialized_view__full_refresh(relation, sql, backup_relation=None, intermediate_relation=None) %}
{% if backup_relation %}
{{ db_api__materialized_view__create(intermediate_relation, sql) }}
{{ adapter.rename_relation(target_relation, backup_relation) }}
{{ adapter.rename_relation(intermediate_relation, target_relation) }}
{{ drop_relation_if_exists(backup_relation) }}
{% else %}
{{ drop_relation_if_exists(target_relation) }}
{{ db_api__materialized_view__create(target_relation, sql) }}
{% endif %}

{% endmacro %}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import pytest

from dbt.tests.util import run_dbt


models__model_sql = """
{{ config(materialized='materialized_view') }}
select 1 as id

"""


@pytest.fixture(scope="class")
def models():
return {"model.sql": models__model_sql}


def test_basic(project):
run_dbt(["run"], expect_pass=False)