From 092c618935df6012bed07710a7eb4366ff240091 Mon Sep 17 00:00:00 2001 From: Rory Sawyer Date: Fri, 2 Feb 2024 11:15:50 -0500 Subject: [PATCH 1/2] fix: use correct schema for MV target tables (#244) * fix: use correct schema when updating MVs The existing implementation passes just the name for `target_table`, which ultimately means that the target schema is not included when the final SQL is generated. By passing the entire relation object, the correct target schema will be present in the final SQL. * update MV tests Provide a custom schema to make sure that the full target table name (schema + relation name) is included in the CREATE MATERIALIZED VIEW statement --- .../macros/materializations/materialized_view.sql | 6 +++--- .../materialized_view/test_materialized_view.py | 11 +++++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/dbt/include/clickhouse/macros/materializations/materialized_view.sql b/dbt/include/clickhouse/macros/materializations/materialized_view.sql index 293cc41b..2ebe0dae 100644 --- a/dbt/include/clickhouse/macros/materializations/materialized_view.sql +++ b/dbt/include/clickhouse/macros/materializations/materialized_view.sql @@ -41,7 +41,7 @@ {{ clickhouse__get_create_materialized_view_as_sql(target_relation, sql) }} {%- endcall %} {% elif existing_relation.can_exchange %} - {{ log('Replacing existing materialized view' + target_relation.name) }} + {{ log('Replacing existing materialized view ' + target_relation.name) }} {% call statement('drop existing materialized view') %} drop view if exists {{ mv_relation }} {{ cluster_clause }} {% endcall %} @@ -50,10 +50,10 @@ {%- endcall %} {% do exchange_tables_atomic(backup_relation, existing_relation) %} {% call statement('create new materialized view') %} - {{ clickhouse__create_mv_sql(mv_relation, existing_relation.name, cluster_clause, sql) }} + {{ clickhouse__create_mv_sql(mv_relation, existing_relation, cluster_clause, sql) }} {% endcall %} {% else %} - {{ log('Replacing existing materialized view' + target_relation.name) }} + {{ log('Replacing existing materialized view ' + target_relation.name) }} {{ clickhouse__replace_mv(target_relation, existing_relation, intermediate_relation, backup_relation, sql) }} {% endif %} diff --git a/tests/integration/adapter/materialized_view/test_materialized_view.py b/tests/integration/adapter/materialized_view/test_materialized_view.py index 9305d064..06b88e6e 100644 --- a/tests/integration/adapter/materialized_view/test_materialized_view.py +++ b/tests/integration/adapter/materialized_view/test_materialized_view.py @@ -25,6 +25,7 @@ materialized='materialized_view', engine='MergeTree()', order_by='(id)', + schema='custom_schema', ) }} {% if var('run_type', '') == '' %} @@ -92,6 +93,7 @@ def test_create(self, project): 2. create a model as a materialized view, selecting from the table created in (1) 3. insert data into the base table and make sure it's there in the target table created in (2) """ + schema = quote_identifier(project.test_schema + "_custom_schema") results = run_dbt(["seed"]) assert len(results) == 1 columns = project.run_sql("DESCRIBE TABLE people", fetch="all") @@ -101,10 +103,10 @@ def test_create(self, project): results = run_dbt() assert len(results) == 1 - columns = project.run_sql("DESCRIBE TABLE hackers", fetch="all") + columns = project.run_sql(f"DESCRIBE TABLE {schema}.hackers", fetch="all") assert columns[0][1] == "Int32" - columns = project.run_sql("DESCRIBE hackers_mv", fetch="all") + columns = project.run_sql(f"DESCRIBE {schema}.hackers_mv", fetch="all") assert columns[0][1] == "Int32" check_relation_types( @@ -123,7 +125,7 @@ def test_create(self, project): """ ) - result = project.run_sql("select count(*) from hackers", fetch="all") + result = project.run_sql(f"select count(*) from {schema}.hackers", fetch="all") assert result[0][0] == 4 @@ -145,6 +147,7 @@ def models(self): } def test_update(self, project): + schema = quote_identifier(project.test_schema + "_custom_schema") # create our initial materialized view run_dbt(["seed"]) run_dbt() @@ -162,6 +165,6 @@ def test_update(self, project): # assert that we now have both of Dade's aliases in our hackers table result = project.run_sql( - "select distinct hacker_alias from hackers where name = 'Dade'", fetch="all" + f"select distinct hacker_alias from {schema}.hackers where name = 'Dade'", fetch="all" ) assert len(result) == 2 From febc2208965476c1e042e29c56893c319163b9db Mon Sep 17 00:00:00 2001 From: Geoff Genz Date: Fri, 2 Feb 2024 09:26:40 -0700 Subject: [PATCH 2/2] Update changelog --- CHANGELOG.md | 5 +++++ README.md | 3 +-- dbt/adapters/clickhouse/__version__.py | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35f64950..1cbef238 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +### Release [1.7.2], 2024-02-09 +#### Bug Fix +- Fixed an issue where Materialize Views would break with a custom schema. Thanks to [Rory Sawyer](https://github.com/SoryRawyer) +for the PR! + ### Release [1.7.1], 2023-12-13 #### Bug Fixes - Some models with LIMIT clauses were broken in recent releases. This has been fixed. Thanks to diff --git a/README.md b/README.md index 8022f214..8e413f67 100644 --- a/README.md +++ b/README.md @@ -156,8 +156,7 @@ operations, because they don't require rewriting ClickHouse data parts. The inc incremental materializations that perform significantly better than the "legacy" strategy. However, there are important caveats to using this strategy: - Lightweight deletes must be enabled on your ClickHouse server using the setting `allow_experimental_lightweight_delete=1` or you must set `use_lw_deletes=true` in your profile (which will enable that setting for your dbt sessions) -- As suggested by the setting name, lightweight delete functionality is still experimental and there are still known issues that must be resolved before the feature is considered production ready, -so usage should be limited to datasets that are easily recreated +- Lightweight deletes are now production ready, but there may be performance and other problems on ClickHouse versions earlier than 23.3. - This strategy operates directly on the affected table/relation (with creating any intermediate or temporary tables), so if there is an issue during the operation, the data in the incremental model is likely to be in an invalid state - When using lightweight deletes, dbt-clickhouse enabled the setting `allow_nondeterministic_mutations`. In some very rare cases using non-deterministic incremental_predicates diff --git a/dbt/adapters/clickhouse/__version__.py b/dbt/adapters/clickhouse/__version__.py index 1f796f9b..41aad93f 100644 --- a/dbt/adapters/clickhouse/__version__.py +++ b/dbt/adapters/clickhouse/__version__.py @@ -1 +1 @@ -version = '1.7.1' +version = '1.7.2'