From bf4995e4dadacfca155189855cfe72926df61353 Mon Sep 17 00:00:00 2001 From: Dennis Hume Date: Mon, 6 Nov 2023 10:22:02 -0600 Subject: [PATCH 1/6] Store Failures As --- .../macros/materializations/test.sql | 13 +- .../tests/adapter/test_store_test_failures.py | 135 ++++++++++++++++++ 2 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 misc/dbt-materialize/tests/adapter/test_store_test_failures.py diff --git a/misc/dbt-materialize/dbt/include/materialize/macros/materializations/test.sql b/misc/dbt-materialize/dbt/include/materialize/macros/materializations/test.sql index 488a7da72f1be..409636b6507b5 100644 --- a/misc/dbt-materialize/dbt/include/materialize/macros/materializations/test.sql +++ b/misc/dbt-materialize/dbt/include/materialize/macros/materializations/test.sql @@ -19,10 +19,21 @@ {% if should_store_failures() %} + {% set store_failures_as = config.get('store_failures_as') %} + -- if `--store-failures` is invoked via command line and `store_failures_as` is not set, + -- config.get('store_failures_as', 'table') returns None, not 'table' + {% if store_failures_as == none %}{% set store_failures_as = 'table' %}{% endif %} + {% if store_failures_as not in ['table', 'view', 'materialized_view'] %} + {{ exceptions.raise_compiler_error( + "'" ~ store_failures_as ~ "' is not a valid value for `store_failures_as`. " + "Accepted values are: ['ephemeral', 'table', 'view', 'materialized_view']" + ) }} + {% endif %} + {% 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='materializedview') -%} %} + identifier=identifier, schema=schema, database=database, type=store_failures_as) -%} %} {% if old_relation %} {% do adapter.drop_relation(old_relation) %} diff --git a/misc/dbt-materialize/tests/adapter/test_store_test_failures.py b/misc/dbt-materialize/tests/adapter/test_store_test_failures.py new file mode 100644 index 0000000000000..1e32f8f4b3e26 --- /dev/null +++ b/misc/dbt-materialize/tests/adapter/test_store_test_failures.py @@ -0,0 +1,135 @@ +# Copyright Materialize, Inc. and contributors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License in the LICENSE file at the +# root of this repository, or online at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import pytest +from dbt.contracts.results import TestStatus +from dbt.tests.adapter.store_test_failures_tests import basic +from dbt.tests.adapter.store_test_failures_tests.fixtures import ( + models__file_model_but_with_a_no_good_very_long_name, + models__fine_model, +) +from dbt.tests.adapter.store_test_failures_tests.test_store_test_failures import ( + StoreTestFailuresBase, +) +from dbt.tests.util import run_dbt + +TEST__MATERIALIZED_VIEW_TRUE = """ +{{ config(store_failures_as="materialized_view", store_failures=True) }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + +TEST__MATERIALIZED_VIEW_FALSE = """ +{{ config(store_failures_as="materialized_view", store_failures=False) }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + +TEST__MATERIALIZED_VIEW_UNSET = """ +{{ config(store_failures_as="materialized_view") }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + +class TestStoreTestFailures(StoreTestFailuresBase): + @pytest.fixture(scope="class") + def models(self): + return { + "fine_model.sql": models__fine_model, + "fine_model_but_with_a_no_good_very_long_name.sql": models__file_model_but_with_a_no_good_very_long_name, + } + + +class TestMaterializeStoreTestFailures(TestStoreTestFailures): + pass + + +class TestStoreTestFailuresAsInteractions(basic.StoreTestFailuresAsInteractions): + pass + + +class TestStoreTestFailuresAsProjectLevelOff(basic.StoreTestFailuresAsProjectLevelOff): + pass + + +class TestStoreTestFailuresAsProjectLevelView( + basic.StoreTestFailuresAsProjectLevelView +): + pass + + +class TestStoreTestFailuresAsGeneric(basic.StoreTestFailuresAsGeneric): + pass + + +class TestStoreTestFailuresAsProjectLevelEphemeral( + basic.StoreTestFailuresAsProjectLevelEphemeral +): + pass + + +class TestStoreTestFailuresAsExceptions(basic.StoreTestFailuresAsExceptions): + def test_tests_run_unsuccessfully_and_raise_appropriate_exception(self, project): + results = run_dbt(["test"], expect_pass=False) + assert len(results) == 1 + result = results[0] + assert "Compilation Error" in result.message + assert "'error' is not a valid value" in result.message + assert ( + "Accepted values are: ['ephemeral', 'table', 'view', 'materialized_view']" + in result.message + ) + + +class TestStoreTestFailuresAsProjectLevelMaterializeView(basic.StoreTestFailuresAsBase): + """ + These scenarios test that `store_failures_as` at the project level takes precedence over `store_failures` + at the model level. + + Test Scenarios: + + - If `store_failures_as = "materialized_view"` in the project and `store_failures = False` in the model, + then store the failures in a materialized view. + - If `store_failures_as = "materialized_view"` in the project and `store_failures = True` in the model, + then store the failures in a materialized view. + - If `store_failures_as = "materialized_view"` in the project and `store_failures` is not set, + then store the failures in a materialized view. + """ + + @pytest.fixture(scope="class") + def tests(self): + return { + "results_true.sql": TEST__MATERIALIZED_VIEW_TRUE, + "results_false.sql": TEST__MATERIALIZED_VIEW_FALSE, + "results_unset.sql": TEST__MATERIALIZED_VIEW_UNSET, + } + + @pytest.fixture(scope="class") + def project_config_update(self): + return {"tests": {"store_failures_as": "materialized_view"}} + + def test_tests_run_successfully_and_are_stored_as_expected(self, project): + expected_results = { + basic.TestResult("results_true", TestStatus.Fail, "materialized_view"), + basic.TestResult("results_false", TestStatus.Fail, "materialized_view"), + basic.TestResult("results_unset", TestStatus.Fail, "materialized_view"), + } + self.run_and_assert(project, expected_results) From 7ae184796044c22d02eb2a21422cd5fae5e4c0cd Mon Sep 17 00:00:00 2001 From: Dennis Hume Date: Thu, 9 Nov 2023 06:58:45 -0600 Subject: [PATCH 2/6] Update create --- misc/dbt-materialize/CHANGELOG.md | 23 +++++++++++++++++++ misc/dbt-materialize/README.md | 4 ++-- .../macros/materializations/test.sql | 16 +++++++++---- .../tests/adapter/test_store_test_failures.py | 2 +- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/misc/dbt-materialize/CHANGELOG.md b/misc/dbt-materialize/CHANGELOG.md index 35e30ce0fefe5..76dff0c6cb0e8 100644 --- a/misc/dbt-materialize/CHANGELOG.md +++ b/misc/dbt-materialize/CHANGELOG.md @@ -7,6 +7,29 @@ checks, but the value of this feature in a real-time data warehouse is limited. +* Support for `store_failures_as` which can be `table`, `view`,`ephemeral` or `materialized_view` (like models, a `table` will be a `materialized_view`). + + * Project level + ```yaml + tests: + my_project: + +store_failures_as: table + ``` + * Model level + ```yaml + models: + - name: my_model + columns: + - name: id + tests: + - not_null: + config: + store_failures_as: view + - unique: + config: + store_failures_as: materialized_view + ``` + ## 1.6.1 - 2023-11-03 * Support the [`ASSERT NOT NULL` option](https://materialize.com/docs/sql/create-materialized-view/#non-null-assertions) diff --git a/misc/dbt-materialize/README.md b/misc/dbt-materialize/README.md index a9ef37cb3f243..dad4a4ef5e53b 100644 --- a/misc/dbt-materialize/README.md +++ b/misc/dbt-materialize/README.md @@ -110,8 +110,8 @@ as well as [`--persist-docs`](https://docs.getdbt.com/reference/resource-configs [`dbt test`](https://docs.getdbt.com/reference/commands/test) is supported. -If you set the optional `--store-failures` flag or [`store-failures` config](https://docs.getdbt.com/reference/resource-configs/store_failures), -dbt will save the results of a test query to a `materialized_view`. These will +If you set the optional flags `--store-failures` or `--store-failures-as` (or config [`store_failures` config](https://docs.getdbt.com/reference/resource-configs/store_failures) or [`store_failures_as` config](https://docs.getdbt.com/reference/resource-configs/store_failures_as)), +By default dbt will save the results of a test query to a `materialized_view`. These will be created in a schema suffixed or named `dbt_test__audit` by default. Change this value by setting a `schema` config. diff --git a/misc/dbt-materialize/dbt/include/materialize/macros/materializations/test.sql b/misc/dbt-materialize/dbt/include/materialize/macros/materializations/test.sql index 409636b6507b5..ab2ac5fdcb25d 100644 --- a/misc/dbt-materialize/dbt/include/materialize/macros/materializations/test.sql +++ b/misc/dbt-materialize/dbt/include/materialize/macros/materializations/test.sql @@ -26,7 +26,7 @@ {% if store_failures_as not in ['table', 'view', 'materialized_view'] %} {{ exceptions.raise_compiler_error( "'" ~ store_failures_as ~ "' is not a valid value for `store_failures_as`. " - "Accepted values are: ['ephemeral', 'table', 'view', 'materialized_view']" + "Accepted values are: ['table', 'view', 'materialized_view']" ) }} {% endif %} @@ -39,12 +39,18 @@ {% do adapter.drop_relation(old_relation) %} {% endif %} - {% call statement(auto_begin=True) %} - {{ materialize__create_materialized_view_as(target_relation, sql) }} - {% endcall %} - {% do relations.append(target_relation) %} + {% if store_failures_as == 'view' %} + {% call statement(auto_begin=True) %} + {{ materialize__create_view_as(target_relation, sql) }} + {% endcall %} + {% else %} + {% call statement(auto_begin=True) %} + {{ materialize__create_materialized_view_as(target_relation, sql) }} + {% endcall %} + {% endif %} + {% set main_sql %} select * from {{ target_relation }} diff --git a/misc/dbt-materialize/tests/adapter/test_store_test_failures.py b/misc/dbt-materialize/tests/adapter/test_store_test_failures.py index 1e32f8f4b3e26..fdbd733f3b6a6 100644 --- a/misc/dbt-materialize/tests/adapter/test_store_test_failures.py +++ b/misc/dbt-materialize/tests/adapter/test_store_test_failures.py @@ -94,7 +94,7 @@ def test_tests_run_unsuccessfully_and_raise_appropriate_exception(self, project) assert "Compilation Error" in result.message assert "'error' is not a valid value" in result.message assert ( - "Accepted values are: ['ephemeral', 'table', 'view', 'materialized_view']" + "Accepted values are: ['table', 'view', 'materialized_view']" in result.message ) From 01798eb92bfb7fe3b20e31e8c724d10766c02607 Mon Sep 17 00:00:00 2001 From: morsapaes Date: Thu, 16 Nov 2023 14:14:35 +0100 Subject: [PATCH 3/6] Minor tweaks, possibly breaking the tests (on purpose) --- misc/dbt-materialize/CHANGELOG.md | 26 ++++++++++++------- .../macros/materializations/test.sql | 17 ++++++------ 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/misc/dbt-materialize/CHANGELOG.md b/misc/dbt-materialize/CHANGELOG.md index 76dff0c6cb0e8..928965f819ff6 100644 --- a/misc/dbt-materialize/CHANGELOG.md +++ b/misc/dbt-materialize/CHANGELOG.md @@ -2,20 +2,18 @@ ## Unreleased -* Mark `dbt source freshness` as not supported. Materialize supports the - functionality required to enable column- and metadata-based source freshness - checks, but the value of this feature in a real-time data warehouse is - limited. - -* Support for `store_failures_as` which can be `table`, `view`,`ephemeral` or `materialized_view` (like models, a `table` will be a `materialized_view`). +* Support specifying the materialization type used to store test failures via + the new [`store_failures_as` configuration](https://docs.getdbt.com/reference/resource-configs/store_failures_as). + Accepted values: `materialized_view` (default), `view`, `ephemeral`. - * Project level + * **Project level** ```yaml tests: my_project: - +store_failures_as: table + +store_failures_as: view ``` - * Model level + + * **Model level** ```yaml models: - name: my_model @@ -27,9 +25,17 @@ store_failures_as: view - unique: config: - store_failures_as: materialized_view + store_failures_as: ephemeral ``` + If both [`store_failures`](https://docs.getdbt.com/reference/resource-configs/store_failures) + and `store_failures_as` are specified, `store_failures_as` takes precedence. + +* Mark `dbt source freshness` as not supported. Materialize supports the + functionality required to enable column- and metadata-based source freshness + checks, but the value of this feature in a real-time data warehouse is + limited. + ## 1.6.1 - 2023-11-03 * Support the [`ASSERT NOT NULL` option](https://materialize.com/docs/sql/create-materialized-view/#non-null-assertions) diff --git a/misc/dbt-materialize/dbt/include/materialize/macros/materializations/test.sql b/misc/dbt-materialize/dbt/include/materialize/macros/materializations/test.sql index ab2ac5fdcb25d..219e06c49950a 100644 --- a/misc/dbt-materialize/dbt/include/materialize/macros/materializations/test.sql +++ b/misc/dbt-materialize/dbt/include/materialize/macros/materializations/test.sql @@ -17,21 +17,22 @@ {% set relations = [] %} + -- For an overview of the precedence logic behind store_failures and + -- store_failures_at, see dbt-core #8653. {% if should_store_failures() %} + {% set identifier = model['alias'] %} + {% set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} + {% set store_failures_as = config.get('store_failures_as') %} - -- if `--store-failures` is invoked via command line and `store_failures_as` is not set, - -- config.get('store_failures_as', 'table') returns None, not 'table' - {% if store_failures_as == none %}{% set store_failures_as = 'table' %}{% endif %} + {% if store_failures_as == none %}{% set store_failures_as = 'materialized_view' %}{% endif %} {% if store_failures_as not in ['table', 'view', 'materialized_view'] %} {{ exceptions.raise_compiler_error( "'" ~ store_failures_as ~ "' is not a valid value for `store_failures_as`. " - "Accepted values are: ['table', 'view', 'materialized_view']" + "Accepted values are: ['ephemeral', 'table', 'view', 'materialized_view']" ) }} {% endif %} - {% 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=store_failures_as) -%} %} @@ -39,8 +40,6 @@ {% do adapter.drop_relation(old_relation) %} {% endif %} - {% do relations.append(target_relation) %} - {% if store_failures_as == 'view' %} {% call statement(auto_begin=True) %} {{ materialize__create_view_as(target_relation, sql) }} @@ -51,6 +50,8 @@ {% endcall %} {% endif %} + {% do relations.append(target_relation) %} + {% set main_sql %} select * from {{ target_relation }} From 2cce1bbf409000659448724eb96b917b9a640ca3 Mon Sep 17 00:00:00 2001 From: morsapaes Date: Fri, 17 Nov 2023 17:51:40 +0100 Subject: [PATCH 4/6] Fixes --- misc/dbt-materialize/README.md | 12 ++++++++---- .../materialize/macros/materializations/test.sql | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/misc/dbt-materialize/README.md b/misc/dbt-materialize/README.md index dad4a4ef5e53b..f98845505543c 100644 --- a/misc/dbt-materialize/README.md +++ b/misc/dbt-materialize/README.md @@ -110,10 +110,14 @@ as well as [`--persist-docs`](https://docs.getdbt.com/reference/resource-configs [`dbt test`](https://docs.getdbt.com/reference/commands/test) is supported. -If you set the optional flags `--store-failures` or `--store-failures-as` (or config [`store_failures` config](https://docs.getdbt.com/reference/resource-configs/store_failures) or [`store_failures_as` config](https://docs.getdbt.com/reference/resource-configs/store_failures_as)), -By default dbt will save the results of a test query to a `materialized_view`. These will -be created in a schema suffixed or named `dbt_test__audit` by default. Change -this value by setting a `schema` config. +If you set the optional [`--store-failures` flag or `store-failures` config](https://docs.getdbt.com/reference/resource-configs/store_failures), +dbt will save the results of a test query to a `materialized_view`. To use a +`view` instead, use the [`store_failures_as` config](https://docs.getdbt.com/reference/resource-configs/store_failures_as). + +These objects will be created in a schema suffixed or named `dbt_test__audit` by +default. Change this value by setting a `schema` config. If both +[`store_failures`](https://docs.getdbt.com/reference/resource-configs/store_failures) and +`store_failures_as` are specified, `store_failures_as` takes precedence. ### Snapshots diff --git a/misc/dbt-materialize/dbt/include/materialize/macros/materializations/test.sql b/misc/dbt-materialize/dbt/include/materialize/macros/materializations/test.sql index 219e06c49950a..f524d0ccb4daf 100644 --- a/misc/dbt-materialize/dbt/include/materialize/macros/materializations/test.sql +++ b/misc/dbt-materialize/dbt/include/materialize/macros/materializations/test.sql @@ -25,7 +25,7 @@ {% set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} {% set store_failures_as = config.get('store_failures_as') %} - {% if store_failures_as == none %}{% set store_failures_as = 'materialized_view' %}{% endif %} + {% if store_failures_as == none %}{% set store_failures_as = 'table' %}{% endif %} {% if store_failures_as not in ['table', 'view', 'materialized_view'] %} {{ exceptions.raise_compiler_error( "'" ~ store_failures_as ~ "' is not a valid value for `store_failures_as`. " From 7a2ac7ed68a3f2225a84998c51dc79fbf8a4b66b Mon Sep 17 00:00:00 2001 From: morsapaes Date: Mon, 20 Nov 2023 11:05:44 +0100 Subject: [PATCH 5/6] Add note to docs --- doc/user/content/manage/dbt.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/user/content/manage/dbt.md b/doc/user/content/manage/dbt.md index aab18f0eb8f0c..88c85ad7fd9cf 100644 --- a/doc/user/content/manage/dbt.md +++ b/doc/user/content/manage/dbt.md @@ -414,8 +414,8 @@ That's it! From here on, Materialize makes sure that your models are **increment ## Test and document a dbt project -[//]: # "TODO(morsapaes) Call out the cluster configuration for tests once this -page is rehashed." +[//]: # "TODO(morsapaes) Call out the cluster configuration for tests and +store_failures_as once this page is rehashed." ### Configure continuous testing From 42de18000c4764475fd7b7da7b911e7fb707fae8 Mon Sep 17 00:00:00 2001 From: morsapaes Date: Mon, 20 Nov 2023 11:45:19 +0100 Subject: [PATCH 6/6] Patch assertion in TestStoreTestFailuresAsExceptions --- .../tests/adapter/test_store_test_failures.py | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/misc/dbt-materialize/tests/adapter/test_store_test_failures.py b/misc/dbt-materialize/tests/adapter/test_store_test_failures.py index fdbd733f3b6a6..36e6c6c2fb959 100644 --- a/misc/dbt-materialize/tests/adapter/test_store_test_failures.py +++ b/misc/dbt-materialize/tests/adapter/test_store_test_failures.py @@ -15,7 +15,16 @@ import pytest from dbt.contracts.results import TestStatus -from dbt.tests.adapter.store_test_failures_tests import basic +from dbt.tests.adapter.store_test_failures_tests.basic import ( + StoreTestFailuresAsBase, + StoreTestFailuresAsExceptions, + StoreTestFailuresAsGeneric, + StoreTestFailuresAsInteractions, + StoreTestFailuresAsProjectLevelEphemeral, + StoreTestFailuresAsProjectLevelOff, + StoreTestFailuresAsProjectLevelView, + TestResult, +) from dbt.tests.adapter.store_test_failures_tests.fixtures import ( models__file_model_but_with_a_no_good_very_long_name, models__fine_model, @@ -62,31 +71,29 @@ class TestMaterializeStoreTestFailures(TestStoreTestFailures): pass -class TestStoreTestFailuresAsInteractions(basic.StoreTestFailuresAsInteractions): +class TestStoreTestFailuresAsInteractions(StoreTestFailuresAsInteractions): pass -class TestStoreTestFailuresAsProjectLevelOff(basic.StoreTestFailuresAsProjectLevelOff): +class TestStoreTestFailuresAsProjectLevelOff(StoreTestFailuresAsProjectLevelOff): pass -class TestStoreTestFailuresAsProjectLevelView( - basic.StoreTestFailuresAsProjectLevelView -): +class TestStoreTestFailuresAsProjectLevelView(StoreTestFailuresAsProjectLevelView): pass -class TestStoreTestFailuresAsGeneric(basic.StoreTestFailuresAsGeneric): +class TestStoreTestFailuresAsGeneric(StoreTestFailuresAsGeneric): pass class TestStoreTestFailuresAsProjectLevelEphemeral( - basic.StoreTestFailuresAsProjectLevelEphemeral + StoreTestFailuresAsProjectLevelEphemeral ): pass -class TestStoreTestFailuresAsExceptions(basic.StoreTestFailuresAsExceptions): +class TestStoreTestFailuresAsExceptions(StoreTestFailuresAsExceptions): def test_tests_run_unsuccessfully_and_raise_appropriate_exception(self, project): results = run_dbt(["test"], expect_pass=False) assert len(results) == 1 @@ -94,12 +101,12 @@ def test_tests_run_unsuccessfully_and_raise_appropriate_exception(self, project) assert "Compilation Error" in result.message assert "'error' is not a valid value" in result.message assert ( - "Accepted values are: ['table', 'view', 'materialized_view']" + "Accepted values are: ['ephemeral', 'table', 'view', 'materialized_view']" in result.message ) -class TestStoreTestFailuresAsProjectLevelMaterializeView(basic.StoreTestFailuresAsBase): +class TestStoreTestFailuresAsProjectLevelMaterializeView(StoreTestFailuresAsBase): """ These scenarios test that `store_failures_as` at the project level takes precedence over `store_failures` at the model level. @@ -128,8 +135,8 @@ def project_config_update(self): def test_tests_run_successfully_and_are_stored_as_expected(self, project): expected_results = { - basic.TestResult("results_true", TestStatus.Fail, "materialized_view"), - basic.TestResult("results_false", TestStatus.Fail, "materialized_view"), - basic.TestResult("results_unset", TestStatus.Fail, "materialized_view"), + TestResult("results_true", TestStatus.Fail, "materialized_view"), + TestResult("results_false", TestStatus.Fail, "materialized_view"), + TestResult("results_unset", TestStatus.Fail, "materialized_view"), } self.run_and_assert(project, expected_results)