From c9b746dba848723fc7682b3bdb2b24c2dcd64e8b Mon Sep 17 00:00:00 2001 From: kubikb Date: Wed, 5 Feb 2025 17:50:19 +0100 Subject: [PATCH 1/5] Enable authorization of materialized views --- .../unreleased/Fixes-20250202-105422.yaml | 6 +++ .../relations/materialized_view/create.sql | 6 +++ .../relations/materialized_view/replace.sql | 6 +++ .../adapter/test_grant_access_to.py | 41 +++++++++++++++++-- 4 files changed, 56 insertions(+), 3 deletions(-) create mode 100644 dbt-bigquery/.changes/unreleased/Fixes-20250202-105422.yaml diff --git a/dbt-bigquery/.changes/unreleased/Fixes-20250202-105422.yaml b/dbt-bigquery/.changes/unreleased/Fixes-20250202-105422.yaml new file mode 100644 index 000000000..af8c00760 --- /dev/null +++ b/dbt-bigquery/.changes/unreleased/Fixes-20250202-105422.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Fix silently ignoring materialized view authorization +time: 2025-02-02T10:54:22.12198+01:00 +custom: + Author: kubikb + Issue: "1471" diff --git a/dbt-bigquery/src/dbt/include/bigquery/macros/relations/materialized_view/create.sql b/dbt-bigquery/src/dbt/include/bigquery/macros/relations/materialized_view/create.sql index d3e8c7685..13eaac666 100644 --- a/dbt-bigquery/src/dbt/include/bigquery/macros/relations/materialized_view/create.sql +++ b/dbt-bigquery/src/dbt/include/bigquery/macros/relations/materialized_view/create.sql @@ -2,6 +2,12 @@ {%- set materialized_view = adapter.Relation.materialized_view_from_relation_config(config.model) -%} + {% if config.get('grant_access_to') %} + {% for grant_target_dict in config.get('grant_access_to') %} + {% do adapter.grant_access_to(this, 'view', None, grant_target_dict) %} + {% endfor %} + {% endif %} + create materialized view if not exists {{ relation }} {% if materialized_view.partition %}{{ partition_by(materialized_view.partition) }}{% endif %} {% if materialized_view.cluster %}{{ cluster_by(materialized_view.cluster.fields) }}{% endif %} diff --git a/dbt-bigquery/src/dbt/include/bigquery/macros/relations/materialized_view/replace.sql b/dbt-bigquery/src/dbt/include/bigquery/macros/relations/materialized_view/replace.sql index 2e4a0b69f..5be66443c 100644 --- a/dbt-bigquery/src/dbt/include/bigquery/macros/relations/materialized_view/replace.sql +++ b/dbt-bigquery/src/dbt/include/bigquery/macros/relations/materialized_view/replace.sql @@ -2,6 +2,12 @@ {%- set materialized_view = adapter.Relation.materialized_view_from_relation_config(config.model) -%} + {% if config.get('grant_access_to') %} + {% for grant_target_dict in config.get('grant_access_to') %} + {% do adapter.grant_access_to(this, 'view', None, grant_target_dict) %} + {% endfor %} + {% endif %} + create or replace materialized view if not exists {{ relation }} {% if materialized_view.partition %}{{ partition_by(materialized_view.partition) }}{% endif %} {% if materialized_view.cluster %}{{ cluster_by(materialized_view.cluster.fields) }}{% endif %} diff --git a/dbt-bigquery/tests/functional/adapter/test_grant_access_to.py b/dbt-bigquery/tests/functional/adapter/test_grant_access_to.py index 633cebe92..08c17cda1 100644 --- a/dbt-bigquery/tests/functional/adapter/test_grant_access_to.py +++ b/dbt-bigquery/tests/functional/adapter/test_grant_access_to.py @@ -21,6 +21,24 @@ def select_1(dataset: str, materialized: str): ) +def select_1_materialized_view(dataset: str): + config = f"""config( + materialized='materialized_view', + grant_access_to=[ + {{'project': 'dbt-test-env', 'dataset': '{dataset}'}}, + ] + )""" + return ( + "{{" + + config + + "}}" + + """ + SELECT one, COUNT(1) AS count_one + FROM {{ ref('select_1_table') }} + GROUP BY one""" + ) + + BAD_CONFIG_TABLE_NAME = "bad_view" BAD_CONFIG_TABLE = """ {{ config( @@ -75,16 +93,33 @@ def models(self, unique_schema): return { "select_1.sql": select_1(dataset=dataset, materialized="view"), "select_1_table.sql": select_1(dataset=dataset, materialized="table"), + "select_1_materialized_view.sql": select_1_materialized_view(dataset=dataset), } - def test_grant_access_succeeds(self, project, setup_grant_schema, teardown_grant_schema): + def test_grant_access_succeeds(self, project, setup_grant_schema, teardown_grant_schema, unique_schema): # Need to run twice to validate idempotency results = run_dbt(["run"]) - assert len(results) == 2 + assert len(results) == 3 time.sleep(10) - results = run_dbt(["run"]) + # Materialized view excluded since it would produce an error since source table is replaced + results = run_dbt(["run", "--exclude", "select_1_materialized_view"]) assert len(results) == 2 + with project.adapter.connection_named("__test_grants"): + client = project.adapter.connections.get_thread_connection().handle + dataset_name = get_schema_name(unique_schema) + dataset_id = "{}.{}".format("dbt-test-env", dataset_name) + bq_dataset = client.get_dataset(dataset_id) + + authorized_view_names = [] + for access_entry in bq_dataset.access_entries: + if access_entry.entity_type != "view": + continue + + authorized_view_names.append(access_entry.entity_id["tableId"]) + + assert set(authorized_view_names) == set(["select_1", "select_1_materialized_view"]) + class TestAccessGrantFails: @pytest.fixture(scope="class") From 92227aab4f32fbb862451f0d83aa88184912d35d Mon Sep 17 00:00:00 2001 From: kubikb Date: Wed, 5 Feb 2025 17:54:03 +0100 Subject: [PATCH 2/5] Python style fix --- dbt-bigquery/tests/functional/adapter/test_grant_access_to.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dbt-bigquery/tests/functional/adapter/test_grant_access_to.py b/dbt-bigquery/tests/functional/adapter/test_grant_access_to.py index 08c17cda1..e9c209a69 100644 --- a/dbt-bigquery/tests/functional/adapter/test_grant_access_to.py +++ b/dbt-bigquery/tests/functional/adapter/test_grant_access_to.py @@ -96,7 +96,9 @@ def models(self, unique_schema): "select_1_materialized_view.sql": select_1_materialized_view(dataset=dataset), } - def test_grant_access_succeeds(self, project, setup_grant_schema, teardown_grant_schema, unique_schema): + def test_grant_access_succeeds( + self, project, setup_grant_schema, teardown_grant_schema, unique_schema + ): # Need to run twice to validate idempotency results = run_dbt(["run"]) assert len(results) == 3 From 34c6434cb5cc4839946c14fe4f112bb4d99200b1 Mon Sep 17 00:00:00 2001 From: kubikb Date: Thu, 6 Feb 2025 06:34:56 +0100 Subject: [PATCH 3/5] Move grant_access_to operation to materialized view materialization itself --- .../macros/materializations/models/materialized_view.sql | 6 ++++++ .../bigquery/macros/relations/materialized_view/create.sql | 6 ------ .../bigquery/macros/relations/materialized_view/replace.sql | 6 ------ 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/dbt-adapters/src/dbt/include/global_project/macros/materializations/models/materialized_view.sql b/dbt-adapters/src/dbt/include/global_project/macros/materializations/models/materialized_view.sql index a39f8aa21..7133180ee 100644 --- a/dbt-adapters/src/dbt/include/global_project/macros/materializations/models/materialized_view.sql +++ b/dbt-adapters/src/dbt/include/global_project/macros/materializations/models/materialized_view.sql @@ -17,6 +17,12 @@ {{ materialized_view_teardown(backup_relation, intermediate_relation, post_hooks) }} + {% if config.get('grant_access_to') %} + {% for grant_target_dict in config.get('grant_access_to') %} + {% do adapter.grant_access_to(this, 'view', None, grant_target_dict) %} + {% endfor %} + {% endif %} + {{ return({'relations': [target_relation]}) }} {% endmaterialization %} diff --git a/dbt-bigquery/src/dbt/include/bigquery/macros/relations/materialized_view/create.sql b/dbt-bigquery/src/dbt/include/bigquery/macros/relations/materialized_view/create.sql index 13eaac666..d3e8c7685 100644 --- a/dbt-bigquery/src/dbt/include/bigquery/macros/relations/materialized_view/create.sql +++ b/dbt-bigquery/src/dbt/include/bigquery/macros/relations/materialized_view/create.sql @@ -2,12 +2,6 @@ {%- set materialized_view = adapter.Relation.materialized_view_from_relation_config(config.model) -%} - {% if config.get('grant_access_to') %} - {% for grant_target_dict in config.get('grant_access_to') %} - {% do adapter.grant_access_to(this, 'view', None, grant_target_dict) %} - {% endfor %} - {% endif %} - create materialized view if not exists {{ relation }} {% if materialized_view.partition %}{{ partition_by(materialized_view.partition) }}{% endif %} {% if materialized_view.cluster %}{{ cluster_by(materialized_view.cluster.fields) }}{% endif %} diff --git a/dbt-bigquery/src/dbt/include/bigquery/macros/relations/materialized_view/replace.sql b/dbt-bigquery/src/dbt/include/bigquery/macros/relations/materialized_view/replace.sql index 5be66443c..2e4a0b69f 100644 --- a/dbt-bigquery/src/dbt/include/bigquery/macros/relations/materialized_view/replace.sql +++ b/dbt-bigquery/src/dbt/include/bigquery/macros/relations/materialized_view/replace.sql @@ -2,12 +2,6 @@ {%- set materialized_view = adapter.Relation.materialized_view_from_relation_config(config.model) -%} - {% if config.get('grant_access_to') %} - {% for grant_target_dict in config.get('grant_access_to') %} - {% do adapter.grant_access_to(this, 'view', None, grant_target_dict) %} - {% endfor %} - {% endif %} - create or replace materialized view if not exists {{ relation }} {% if materialized_view.partition %}{{ partition_by(materialized_view.partition) }}{% endif %} {% if materialized_view.cluster %}{{ cluster_by(materialized_view.cluster.fields) }}{% endif %} From 865a98f449c3e7d93bbf81ec880472292c6d6270 Mon Sep 17 00:00:00 2001 From: kubikb Date: Thu, 6 Feb 2025 06:35:27 +0100 Subject: [PATCH 4/5] Stylefix --- dbt-bigquery/tests/functional/adapter/test_grant_access_to.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbt-bigquery/tests/functional/adapter/test_grant_access_to.py b/dbt-bigquery/tests/functional/adapter/test_grant_access_to.py index e9c209a69..57edd13f8 100644 --- a/dbt-bigquery/tests/functional/adapter/test_grant_access_to.py +++ b/dbt-bigquery/tests/functional/adapter/test_grant_access_to.py @@ -97,7 +97,7 @@ def models(self, unique_schema): } def test_grant_access_succeeds( - self, project, setup_grant_schema, teardown_grant_schema, unique_schema + self, project, setup_grant_schema, teardown_grant_schema, unique_schema ): # Need to run twice to validate idempotency results = run_dbt(["run"]) From 42dfc9aaf7bfa23eac96546e7fa273449ebcc527 Mon Sep 17 00:00:00 2001 From: kubikb Date: Thu, 6 Feb 2025 07:07:58 +0100 Subject: [PATCH 5/5] Add custom BQ materialized_view materialization instead --- .../materializations/models/materialized_view.sql | 6 ------ .../macros/materializations/materialized_view.sql | 13 +++++++++++++ 2 files changed, 13 insertions(+), 6 deletions(-) create mode 100644 dbt-bigquery/src/dbt/include/bigquery/macros/materializations/materialized_view.sql diff --git a/dbt-adapters/src/dbt/include/global_project/macros/materializations/models/materialized_view.sql b/dbt-adapters/src/dbt/include/global_project/macros/materializations/models/materialized_view.sql index 7133180ee..a39f8aa21 100644 --- a/dbt-adapters/src/dbt/include/global_project/macros/materializations/models/materialized_view.sql +++ b/dbt-adapters/src/dbt/include/global_project/macros/materializations/models/materialized_view.sql @@ -17,12 +17,6 @@ {{ materialized_view_teardown(backup_relation, intermediate_relation, post_hooks) }} - {% if config.get('grant_access_to') %} - {% for grant_target_dict in config.get('grant_access_to') %} - {% do adapter.grant_access_to(this, 'view', None, grant_target_dict) %} - {% endfor %} - {% endif %} - {{ return({'relations': [target_relation]}) }} {% endmaterialization %} diff --git a/dbt-bigquery/src/dbt/include/bigquery/macros/materializations/materialized_view.sql b/dbt-bigquery/src/dbt/include/bigquery/macros/materializations/materialized_view.sql new file mode 100644 index 000000000..0b305cc5b --- /dev/null +++ b/dbt-bigquery/src/dbt/include/bigquery/macros/materializations/materialized_view.sql @@ -0,0 +1,13 @@ +{% materialization materialized_view, adapter='bigquery' -%} + + {% set relations = materialization_materialized_view_default() %} + + {% if config.get('grant_access_to') %} + {% for grant_target_dict in config.get('grant_access_to') %} + {% do adapter.grant_access_to(this, 'view', None, grant_target_dict) %} + {% endfor %} + {% endif %} + + {{ return(relations) }} + +{%- endmaterialization %}