Skip to content

Commit

Permalink
(fixes #744) deprecate sql_where and provide an alternative
Browse files Browse the repository at this point in the history
  • Loading branch information
drewbanin committed Dec 5, 2018
1 parent bb6b469 commit 80232ff
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 24 deletions.
7 changes: 7 additions & 0 deletions dbt/compilation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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)

Expand Down
13 changes: 12 additions & 1 deletion dbt/deprecations.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from dbt.logger import GLOBAL_LOGGER as logger
import dbt.links


class DBTDeprecation(object):
Expand All @@ -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'
Expand All @@ -50,7 +60,8 @@ def warn(name, *args, **kwargs):

deprecations_list = [
DBTRepositoriesDeprecation(),
SeedDropExistingDeprecation()
SeedDropExistingDeprecation(),
SqlWhereDeprecation(),
]

deprecations = {d.name: d for d in deprecations_list}
Expand Down
10 changes: 10 additions & 0 deletions dbt/include/global_project/macros/etc/is_incremental.sql
Original file line number Diff line number Diff line change
@@ -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 %}
Original file line number Diff line number Diff line change
Expand Up @@ -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 -%}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'] -%}
Expand Down Expand Up @@ -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) }}
Expand Down
1 change: 1 addition & 0 deletions dbt/links.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@

SnowflakeQuotingDocs = 'https://docs.getdbt.com/v0.10/docs/configuring-quoting'
IncrementalDocs = 'https://docs.getdbt.com/docs/configuring-incremental-models'
7 changes: 5 additions & 2 deletions test/integration/001_simple_copy_test/models/incremental.sql
Original file line number Diff line number Diff line change
@@ -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 %}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{{
config(
materialized = "incremental",
sql_where = "id>(select max(id) from {{this}})"
)
}}

select * from {{ ref('seed') }}
40 changes: 21 additions & 19 deletions test/integration/001_simple_copy_test/test_simple_copy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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()

Expand All @@ -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):
Expand All @@ -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")],
Expand All @@ -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):
Expand All @@ -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")

Expand All @@ -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")

Expand All @@ -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):
Expand All @@ -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):
Expand Down

0 comments on commit 80232ff

Please sign in to comment.