Merge pull request #1129 from fishtown-analytics/feature/better-incre…

…mental-models (fixes #744) deprecate sql_where and provide an alternative
dbt-labs · Dec 5, 2018 · 2cd24cf · 2cd24cf
2 parents 963fb84 + 009eaa3
commit 2cd24cf
Show file tree

Hide file tree

Showing 9 changed files with 72 additions and 24 deletions.
diff --git 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
@@ -1,4 +1,5 @@
 from dbt.logger import GLOBAL_LOGGER as logger
+import dbt.links
 
 
 class DBTDeprecation(object):
@@ -26,6 +27,16 @@ class DBTRepositoriesDeprecation(DBTDeprecation):
   """
 
 
+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'
     description = """The --drop-existing argument to `dbt seed` has been
@@ -50,7 +61,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
@@ -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
@@ -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
@@ -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
@@ -1,2 +1,3 @@
 ProfileConfigDocs = 'https://docs.getdbt.com/docs/configure-your-profile'
 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
@@ -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
@@ -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
@@ -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,19 +152,19 @@ 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")],
         })
         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):