From 80232ff8e8e58ab4a061618bbcc8958f2aa2395a Mon Sep 17 00:00:00 2001 From: Drew Banin Date: Wed, 14 Nov 2018 23:06:05 -0500 Subject: [PATCH 1/2] (fixes #744) deprecate sql_where and provide an alternative --- dbt/compilation.py | 7 ++++ dbt/deprecations.py | 13 +++++- .../macros/etc/is_incremental.sql | 10 +++++ .../incremental/bigquery_incremental.sql | 4 +- .../incremental/incremental.sql | 5 ++- dbt/links.py | 1 + .../models/incremental.sql | 7 +++- .../models/incremental_deprecated.sql | 8 ++++ .../001_simple_copy_test/test_simple_copy.py | 40 ++++++++++--------- 9 files changed, 71 insertions(+), 24 deletions(-) create mode 100644 dbt/include/global_project/macros/etc/is_incremental.sql create mode 100644 test/integration/001_simple_copy_test/models/incremental_deprecated.sql diff --git a/dbt/compilation.py b/dbt/compilation.py index f339c5f9152..9563a381cc9 100644 --- a/dbt/compilation.py +++ b/dbt/compilation.py @@ -232,6 +232,12 @@ def _check_resource_uniqueness(cls, manifest): names_resources[name] = node alias_resources[alias] = node + def warn_for_deprecated_configs(self, manifest): + for unique_id, node in manifest.nodes.items(): + is_model = node.resource_type == NodeType.Model + if is_model and 'sql_where' in node.config: + dbt.deprecations.warn('sql_where') + def compile(self): linker = Linker() @@ -247,6 +253,7 @@ def compile(self): disabled_fqns = [n.fqn for n in manifest.disabled] self.config.warn_for_unused_resource_config_paths(resource_fqns, disabled_fqns) + self.warn_for_deprecated_configs(manifest) self.link_graph(linker, manifest) diff --git a/dbt/deprecations.py b/dbt/deprecations.py index 00d656b7be0..eeec80473f5 100644 --- a/dbt/deprecations.py +++ b/dbt/deprecations.py @@ -1,4 +1,5 @@ from dbt.logger import GLOBAL_LOGGER as logger +import dbt.links class DBTDeprecation(object): @@ -25,6 +26,15 @@ class DBTRepositoriesDeprecation(DBTDeprecation): {recommendation} """ +class SqlWhereDeprecation(DBTDeprecation): + name = "sql_where" + description = """\ +The `sql_where` option for incremental models is deprecated and will be + removed in a future release. Check the docs for more information + + {} + """.format(dbt.links.IncrementalDocs) + class SeedDropExistingDeprecation(DBTDeprecation): name = 'drop-existing' @@ -50,7 +60,8 @@ def warn(name, *args, **kwargs): deprecations_list = [ DBTRepositoriesDeprecation(), - SeedDropExistingDeprecation() + SeedDropExistingDeprecation(), + SqlWhereDeprecation(), ] deprecations = {d.name: d for d in deprecations_list} diff --git a/dbt/include/global_project/macros/etc/is_incremental.sql b/dbt/include/global_project/macros/etc/is_incremental.sql new file mode 100644 index 00000000000..fb761cd7c53 --- /dev/null +++ b/dbt/include/global_project/macros/etc/is_incremental.sql @@ -0,0 +1,10 @@ + +{% macro is_incremental() %} + {#-- do not run introspective queries in parsing #} + {% if not execute %} + {{ return(False) }} + {% else %} + {% set relation = adapter.get_relation(this.schema, this.table) %} + {{ return(relation is not none and not flags.FULL_REFRESH) }} + {% endif %} +{% endmacro %} diff --git a/dbt/include/global_project/macros/materializations/incremental/bigquery_incremental.sql b/dbt/include/global_project/macros/materializations/incremental/bigquery_incremental.sql index e24a9caa132..d43b5f7a645 100644 --- a/dbt/include/global_project/macros/materializations/incremental/bigquery_incremental.sql +++ b/dbt/include/global_project/macros/materializations/incremental/bigquery_incremental.sql @@ -37,7 +37,9 @@ select * from ( {{ sql }} ) - where ({{ sql_where }}) or ({{ sql_where }}) is null + {% if sql_where %} + where ({{ sql_where }}) or ({{ sql_where }}) is null + {% endif %} ) {%- endset -%} diff --git a/dbt/include/global_project/macros/materializations/incremental/incremental.sql b/dbt/include/global_project/macros/materializations/incremental/incremental.sql index 8df0264a8a4..ec64645ab9e 100644 --- a/dbt/include/global_project/macros/materializations/incremental/incremental.sql +++ b/dbt/include/global_project/macros/materializations/incremental/incremental.sql @@ -12,7 +12,7 @@ {%- endmacro %} {% materialization incremental, default -%} - {%- set sql_where = config.require('sql_where') -%} + {%- set sql_where = config.get('sql_where') -%} {%- set unique_key = config.get('unique_key') -%} {%- set identifier = model['alias'] -%} @@ -61,8 +61,11 @@ select * from ( {{ sql }} ) as dbt_incr_sbq + + {% if sql_where %} where ({{ sql_where }}) or ({{ sql_where }}) is null + {% endif %} {%- endset %} {{ dbt.create_table_as(True, tmp_relation, tmp_table_sql) }} diff --git a/dbt/links.py b/dbt/links.py index 3be2c275d9e..39fb0d4e0d7 100644 --- a/dbt/links.py +++ b/dbt/links.py @@ -1,2 +1,3 @@ SnowflakeQuotingDocs = 'https://docs.getdbt.com/v0.10/docs/configuring-quoting' +IncrementalDocs = 'https://docs.getdbt.com/docs/configuring-incremental-models' diff --git a/test/integration/001_simple_copy_test/models/incremental.sql b/test/integration/001_simple_copy_test/models/incremental.sql index 72e0c872e6b..020bf35163e 100644 --- a/test/integration/001_simple_copy_test/models/incremental.sql +++ b/test/integration/001_simple_copy_test/models/incremental.sql @@ -1,8 +1,11 @@ {{ config( - materialized = "incremental", - sql_where = "id>(select max(id) from {{this}})" + materialized = "incremental" ) }} select * from {{ ref('seed') }} + +{% if is_incremental() %} + where id > (select max(id) from {{this}}) +{% endif %} diff --git a/test/integration/001_simple_copy_test/models/incremental_deprecated.sql b/test/integration/001_simple_copy_test/models/incremental_deprecated.sql new file mode 100644 index 00000000000..72e0c872e6b --- /dev/null +++ b/test/integration/001_simple_copy_test/models/incremental_deprecated.sql @@ -0,0 +1,8 @@ +{{ + config( + materialized = "incremental", + sql_where = "id>(select max(id) from {{this}})" + ) +}} + +select * from {{ ref('seed') }} diff --git a/test/integration/001_simple_copy_test/test_simple_copy.py b/test/integration/001_simple_copy_test/test_simple_copy.py index 7cc4ed580e4..ea6194a196e 100644 --- a/test/integration/001_simple_copy_test/test_simple_copy.py +++ b/test/integration/001_simple_copy_test/test_simple_copy.py @@ -25,17 +25,17 @@ def test__postgres__simple_copy(self): results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 6) + self.assertEqual(len(results), 7) - self.assertManyTablesEqual(["seed", "view_model", "incremental", "materialized"]) + self.assertManyTablesEqual(["seed", "view_model", "incremental", "incremental_deprecated", "materialized"]) self.use_default_project({"data-paths": [self.dir("seed-update")]}) results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 6) + self.assertEqual(len(results), 7) - self.assertManyTablesEqual(["seed", "view_model", "incremental", "materialized"]) + self.assertManyTablesEqual(["seed", "view_model", "incremental", "incremental_deprecated", "materialized"]) @use_profile("postgres") def test__postgres__dbt_doesnt_run_empty_models(self): @@ -44,7 +44,7 @@ def test__postgres__dbt_doesnt_run_empty_models(self): results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 6) + self.assertEqual(len(results), 7) models = self.get_models_in_schema() @@ -58,13 +58,13 @@ def test__snowflake__simple_copy(self): self.run_dbt(["seed"]) self.run_dbt() - self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "MATERIALIZED"]) + self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "INCREMENTAL_DEPRECATED", "MATERIALIZED"]) self.use_default_project({"data-paths": [self.dir("seed-update")]}) self.run_dbt(["seed"]) self.run_dbt() - self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "MATERIALIZED"]) + self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "INCREMENTAL_DEPRECATED", "MATERIALIZED"]) @use_profile("snowflake") def test__snowflake__simple_copy__quoting_off(self): @@ -76,9 +76,9 @@ def test__snowflake__simple_copy__quoting_off(self): results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 6) + self.assertEqual(len(results), 7) - self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "MATERIALIZED"]) + self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "INCREMENTAL_DEPRECATED", "MATERIALIZED"]) self.use_default_project({ "data-paths": [self.dir("seed-update")], @@ -87,9 +87,9 @@ def test__snowflake__simple_copy__quoting_off(self): results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 6) + self.assertEqual(len(results), 7) - self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "MATERIALIZED"]) + self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "INCREMENTAL_DEPRECATED", "MATERIALIZED"]) @use_profile("snowflake") def test__snowflake__seed__quoting_switch(self): @@ -114,9 +114,10 @@ def test__bigquery__simple_copy(self): results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 6) + self.assertEqual(len(results), 7) self.assertTablesEqual("seed","view_model") + self.assertTablesEqual("seed","incremental_deprecated") self.assertTablesEqual("seed","incremental") self.assertTablesEqual("seed","materialized") @@ -125,9 +126,10 @@ def test__bigquery__simple_copy(self): results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 6) + self.assertEqual(len(results), 7) self.assertTablesEqual("seed","view_model") + self.assertTablesEqual("seed","incremental_deprecated") self.assertTablesEqual("seed","incremental") self.assertTablesEqual("seed","materialized") @@ -150,9 +152,9 @@ def test__snowflake__simple_copy__quoting_on(self): results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 6) + self.assertEqual(len(results), 7) - self.assertManyTablesEqual(["seed", "view_model", "incremental", "materialized"]) + self.assertManyTablesEqual(["seed", "view_model", "incremental", "incremental_deprecated", "materialized"]) self.use_default_project({ "data-paths": [self.dir("seed-update")], @@ -160,9 +162,9 @@ def test__snowflake__simple_copy__quoting_on(self): results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) results = self.run_dbt() - self.assertEqual(len(results), 6) + self.assertEqual(len(results), 7) - self.assertManyTablesEqual(["seed", "view_model", "incremental", "materialized"]) + self.assertManyTablesEqual(["seed", "view_model", "incremental", "incremental_deprecated", "materialized"]) class BaseLowercasedSchemaTest(BaseTestSimpleCopy): @@ -180,13 +182,13 @@ def test__snowflake__simple_copy(self): self.run_dbt(["seed"]) self.run_dbt() - self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "MATERIALIZED"]) + self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "INCREMENTAL_DEPRECATED", "MATERIALIZED"]) self.use_default_project({"data-paths": [self.dir("seed-update")]}) self.run_dbt(["seed"]) self.run_dbt() - self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "MATERIALIZED"]) + self.assertManyTablesEqual(["SEED", "VIEW_MODEL", "INCREMENTAL", "INCREMENTAL_DEPRECATED", "MATERIALIZED"]) class TestSnowflakeSimpleLowercasedSchemaQuoted(BaseLowercasedSchemaTest): From 009eaa3a59a63ddd8808ef378ffc8170c17c1f69 Mon Sep 17 00:00:00 2001 From: Drew Banin Date: Wed, 5 Dec 2018 11:06:04 -0500 Subject: [PATCH 2/2] pep8 --- dbt/deprecations.py | 1 + 1 file changed, 1 insertion(+) diff --git a/dbt/deprecations.py b/dbt/deprecations.py index eeec80473f5..71f9db25495 100644 --- a/dbt/deprecations.py +++ b/dbt/deprecations.py @@ -26,6 +26,7 @@ class DBTRepositoriesDeprecation(DBTDeprecation): {recommendation} """ + class SqlWhereDeprecation(DBTDeprecation): name = "sql_where" description = """\