Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(fixes #744) deprecate sql_where and provide an alternative #1129

Merged
merged 2 commits into from
Dec 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
14 changes: 13 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 Down Expand Up @@ -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
Expand All @@ -50,7 +61,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