From aef7866e294697fc155ea1767f9aaa8e8087d65d Mon Sep 17 00:00:00 2001 From: Drew Banin Date: Tue, 13 Nov 2018 10:36:35 -0500 Subject: [PATCH 01/11] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba69cd617c8..47d4fca789c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## dbt 0.12.0 - Guion Bluford (Currently Unreleased) +## dbt 0.12.0 - Guion Bluford (November 12, 2018) ### Overview From adb1df0904970a1d574808f351ab5c8792f66fbf Mon Sep 17 00:00:00 2001 From: Bastien Boutonnet Date: Sat, 6 Jul 2019 14:07:59 +0200 Subject: [PATCH 02/11] implement snowflake cluster by test to see if changes are taken into account --- .../dbt/include/snowflake/macros/adapters.sql | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql index 4bd3333a7c8..cff4975326a 100644 --- a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql +++ b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql @@ -1,14 +1,25 @@ {% macro snowflake__create_table_as(temporary, relation, sql) -%} {%- set transient = config.get('transient', default=true) -%} - + {%- set cluster_by_keys = config.get('cluster_by') -%} + {# {%- set is_incremental = (config.get('materialization') == 'incremental' and cluster_by_keys is not none) -%} #} + {{log("cluster by keys: {{cluster_by_keys}}")}} + {%- endif -%} create or replace {% if temporary -%} temporary - {%- elif transient -%} - transient + {# {%- elif transient -%} #} + {# transient #} {%- endif %} table {{ relation }} as ( - {{ sql }} + {# {%- if cluster_by_keys is not none -%} #} + select * from( {{ sql }} ) order by ({{ cluster_by_keys }}) + {# {%- else -%} #} + {# {{ sql }} #} + {# {%- endif -%} #} ); +{# if materialisation is incremental configure autoclustering on clustering keys #} + {%- if is_incremental and cluster_by_keys -%} + alter table {{ relation }} cluster by ( {{ cluster_by_keys }}) + {%- endif -%} {% endmacro %} {% macro snowflake__create_view_as(relation, sql) -%} From 32540a1a11dc698adf0f6472f4079aec30e5028d Mon Sep 17 00:00:00 2001 From: Bastien Boutonnet Date: Sat, 6 Jul 2019 16:07:52 +0200 Subject: [PATCH 03/11] add alter statement --- .../dbt/include/snowflake/macros/adapters.sql | 27 ++++++++++--------- .../macros/materializations/incremental.sql | 8 ++++++ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql index cff4975326a..7792fa65e1f 100644 --- a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql +++ b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql @@ -1,25 +1,26 @@ {% macro snowflake__create_table_as(temporary, relation, sql) -%} {%- set transient = config.get('transient', default=true) -%} {%- set cluster_by_keys = config.get('cluster_by') -%} - {# {%- set is_incremental = (config.get('materialization') == 'incremental' and cluster_by_keys is not none) -%} #} - {{log("cluster by keys: {{cluster_by_keys}}")}} - {%- endif -%} + create or replace {% if temporary -%} temporary - {# {%- elif transient -%} #} - {# transient #} + {%- elif transient -%} + transient {%- endif %} table {{ relation }} as ( - {# {%- if cluster_by_keys is not none -%} #} + {%- if cluster_by_keys is not none -%} select * from( {{ sql }} ) order by ({{ cluster_by_keys }}) - {# {%- else -%} #} - {# {{ sql }} #} - {# {%- endif -%} #} + {%- else -%} + {{ sql }} + {%- endif -%} ); -{# if materialisation is incremental configure autoclustering on clustering keys #} - {%- if is_incremental and cluster_by_keys -%} - alter table {{ relation }} cluster by ( {{ cluster_by_keys }}) - {%- endif -%} + + +{% endmacro %} + +{% macro snowflake__alter_cluster(relation, sql) -%} + {%- set cluster_by_keys = config.get('cluster_by') -%} + alter table {{relation}} cluster by ({{cluster_by_keys}}) {% endmacro %} {% macro snowflake__create_view_as(relation, sql) -%} diff --git a/plugins/snowflake/dbt/include/snowflake/macros/materializations/incremental.sql b/plugins/snowflake/dbt/include/snowflake/macros/materializations/incremental.sql index 238fad995f2..f826d2ddf2d 100644 --- a/plugins/snowflake/dbt/include/snowflake/macros/materializations/incremental.sql +++ b/plugins/snowflake/dbt/include/snowflake/macros/materializations/incremental.sql @@ -5,6 +5,8 @@ {%- set full_refresh_mode = (flags.FULL_REFRESH == True) -%} {%- set identifier = model['alias'] -%} + {%- set cluster_by_keys = config.get('cluster_by') -%} + {%- set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) -%} {%- set target_relation = api.Relation.create(database=database, schema=schema, @@ -42,8 +44,14 @@ {%- call statement('main') -%} {{ create_table_as(false, target_relation, sql) }} + + {%- if cluster_by_keys is not none -%} + {{snowflake__alter_cluster(target_relation, sql)}} + {%- endif -%} + {%- endcall -%} + {%- else -%} {%- call statement() -%} From 8fbd792fed6a6d7aa50ec4af9ace3fa15320d652 Mon Sep 17 00:00:00 2001 From: Bastien Boutonnet Date: Sat, 6 Jul 2019 16:17:15 +0200 Subject: [PATCH 04/11] add some todos and fixmes for draft PR --- plugins/snowflake/dbt/include/snowflake/macros/adapters.sql | 2 ++ .../include/snowflake/macros/materializations/incremental.sql | 1 + 2 files changed, 3 insertions(+) diff --git a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql index 7792fa65e1f..9c74213c8de 100644 --- a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql +++ b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql @@ -1,5 +1,6 @@ {% macro snowflake__create_table_as(temporary, relation, sql) -%} {%- set transient = config.get('transient', default=true) -%} + {# TODO: Add support for more than one keys #} {%- set cluster_by_keys = config.get('cluster_by') -%} create or replace {% if temporary -%} @@ -19,6 +20,7 @@ {% endmacro %} {% macro snowflake__alter_cluster(relation, sql) -%} + {# TODO: Add support for more than one keys #} {%- set cluster_by_keys = config.get('cluster_by') -%} alter table {{relation}} cluster by ({{cluster_by_keys}}) {% endmacro %} diff --git a/plugins/snowflake/dbt/include/snowflake/macros/materializations/incremental.sql b/plugins/snowflake/dbt/include/snowflake/macros/materializations/incremental.sql index f826d2ddf2d..acc76efb5c2 100644 --- a/plugins/snowflake/dbt/include/snowflake/macros/materializations/incremental.sql +++ b/plugins/snowflake/dbt/include/snowflake/macros/materializations/incremental.sql @@ -46,6 +46,7 @@ {{ create_table_as(false, target_relation, sql) }} {%- if cluster_by_keys is not none -%} + {# FIXME: Not sure this is the most elegant thing to do... #} {{snowflake__alter_cluster(target_relation, sql)}} {%- endif -%} From 2bc59e06e0ae29e33cef9b0a2d0094ae25d591c7 Mon Sep 17 00:00:00 2001 From: Bastien Boutonnet Date: Sat, 6 Jul 2019 16:18:08 +0200 Subject: [PATCH 05/11] add one more todo --- .../include/snowflake/macros/materializations/incremental.sql | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/snowflake/dbt/include/snowflake/macros/materializations/incremental.sql b/plugins/snowflake/dbt/include/snowflake/macros/materializations/incremental.sql index acc76efb5c2..69c1be0636e 100644 --- a/plugins/snowflake/dbt/include/snowflake/macros/materializations/incremental.sql +++ b/plugins/snowflake/dbt/include/snowflake/macros/materializations/incremental.sql @@ -5,6 +5,7 @@ {%- set full_refresh_mode = (flags.FULL_REFRESH == True) -%} {%- set identifier = model['alias'] -%} + {# TODO: Make sure we can support a list of keys down the line #} {%- set cluster_by_keys = config.get('cluster_by') -%} {%- set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) -%} From 1cabf43924a2ba599359958ea45af681ef1f69fc Mon Sep 17 00:00:00 2001 From: Bastien Boutonnet Date: Sat, 6 Jul 2019 17:32:06 +0200 Subject: [PATCH 06/11] break wrapping lines to avoid being bitten by a one line commen in user sql --- plugins/snowflake/dbt/include/snowflake/macros/adapters.sql | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql index 9c74213c8de..1a0e8e9558c 100644 --- a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql +++ b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql @@ -10,7 +10,9 @@ {%- endif %} table {{ relation }} as ( {%- if cluster_by_keys is not none -%} - select * from( {{ sql }} ) order by ({{ cluster_by_keys }}) + select * from( + {{ sql }} + ) order by ({{ cluster_by_keys }}) {%- else -%} {{ sql }} {%- endif -%} From fe0be06580a899f99f6fba4cff79a22d15d86989 Mon Sep 17 00:00:00 2001 From: Bastien Boutonnet Date: Mon, 8 Jul 2019 14:29:19 +0200 Subject: [PATCH 07/11] bring alter statements in create table and add clustering activation --- .../dbt/include/snowflake/macros/adapters.sql | 43 ++++++++++--------- .../macros/materializations/incremental.sql | 9 ---- 2 files changed, 23 insertions(+), 29 deletions(-) diff --git a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql index 1a0e8e9558c..9c4f576ac97 100644 --- a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql +++ b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql @@ -2,29 +2,32 @@ {%- set transient = config.get('transient', default=true) -%} {# TODO: Add support for more than one keys #} {%- set cluster_by_keys = config.get('cluster_by') -%} - - create or replace {% if temporary -%} - temporary - {%- elif transient -%} - transient - {%- endif %} table {{ relation }} - as ( - {%- if cluster_by_keys is not none -%} - select * from( - {{ sql }} - ) order by ({{ cluster_by_keys }}) - {%- else -%} - {{ sql }} - {%- endif -%} - ); + {%- set is_incremental = config.get('materialized') == 'incremental' -%} + {%- if cluster_by_keys is not string and cluster_by_keys is not none -%} + {%- set cluster_by_keys = cluster_by_keys|join(", ") -%} + {%- endif -%} + + create or replace {% if temporary -%} + temporary + {%- elif transient -%} + transient + {%- endif %} table {{ relation }} + as ( + {%- if cluster_by_keys is not none -%} + select * from( + {{ sql }} + ) order by ({{ cluster_by_keys }}) + {%- else -%} + {{ sql }} + {%- endif %} + ); + {% if cluster_by_keys is not none and is_incremental -%} + alter table {{relation}} cluster by ({{cluster_by_keys}}); + alter table {{relation}} resume recluster; + {%- endif -%} -{% endmacro %} -{% macro snowflake__alter_cluster(relation, sql) -%} - {# TODO: Add support for more than one keys #} - {%- set cluster_by_keys = config.get('cluster_by') -%} - alter table {{relation}} cluster by ({{cluster_by_keys}}) {% endmacro %} {% macro snowflake__create_view_as(relation, sql) -%} diff --git a/plugins/snowflake/dbt/include/snowflake/macros/materializations/incremental.sql b/plugins/snowflake/dbt/include/snowflake/macros/materializations/incremental.sql index 69c1be0636e..f7a26363fa0 100644 --- a/plugins/snowflake/dbt/include/snowflake/macros/materializations/incremental.sql +++ b/plugins/snowflake/dbt/include/snowflake/macros/materializations/incremental.sql @@ -5,9 +5,6 @@ {%- set full_refresh_mode = (flags.FULL_REFRESH == True) -%} {%- set identifier = model['alias'] -%} - {# TODO: Make sure we can support a list of keys down the line #} - {%- set cluster_by_keys = config.get('cluster_by') -%} - {%- set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) -%} {%- set target_relation = api.Relation.create(database=database, schema=schema, @@ -46,14 +43,8 @@ {%- call statement('main') -%} {{ create_table_as(false, target_relation, sql) }} - {%- if cluster_by_keys is not none -%} - {# FIXME: Not sure this is the most elegant thing to do... #} - {{snowflake__alter_cluster(target_relation, sql)}} - {%- endif -%} - {%- endcall -%} - {%- else -%} {%- call statement() -%} From b4fbf7c74262e44ecfda12d899cce6e9a5432804 Mon Sep 17 00:00:00 2001 From: Bastien Boutonnet Date: Sat, 20 Jul 2019 11:22:24 +0200 Subject: [PATCH 08/11] add multiple keys handling --- .../dbt/include/snowflake/macros/adapters.sql | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql index 9c4f576ac97..6b8a6bfdb12 100644 --- a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql +++ b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql @@ -1,10 +1,10 @@ {% macro snowflake__create_table_as(temporary, relation, sql) -%} {%- set transient = config.get('transient', default=true) -%} - {# TODO: Add support for more than one keys #} {%- set cluster_by_keys = config.get('cluster_by') -%} {%- set is_incremental = config.get('materialized') == 'incremental' -%} - {%- if cluster_by_keys is not string and cluster_by_keys is not none -%} - {%- set cluster_by_keys = cluster_by_keys|join(", ") -%} + {%- if cluster_by_keys is not none and cluster_by_keys is string -%} + {%- set cluster_by_keys = [cluster_by_keys] -%} + {%- set cluster_by_string = cluster_by_keys|join(", ")-%} {%- endif -%} create or replace {% if temporary -%} @@ -16,13 +16,13 @@ {%- if cluster_by_keys is not none -%} select * from( {{ sql }} - ) order by ({{ cluster_by_keys }}) + ) order by ({{ cluster_by_string }}) {%- else -%} {{ sql }} {%- endif %} ); {% if cluster_by_keys is not none and is_incremental -%} - alter table {{relation}} cluster by ({{cluster_by_keys}}); + alter table {{relation}} cluster by ({{cluster_by_string}}); alter table {{relation}} resume recluster; {%- endif -%} From fd582976cbe03b66d5266776e384d728b8984759 Mon Sep 17 00:00:00 2001 From: Bastien Boutonnet Date: Sat, 20 Jul 2019 12:37:37 +0200 Subject: [PATCH 09/11] add cluster by and automatic clustering to impl and change tests --- .../snowflake/dbt/adapters/snowflake/impl.py | 23 ++++++++----------- .../dbt/include/snowflake/macros/adapters.sql | 10 +++++--- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/plugins/snowflake/dbt/adapters/snowflake/impl.py b/plugins/snowflake/dbt/adapters/snowflake/impl.py index c6df92f2ee0..eda01763368 100644 --- a/plugins/snowflake/dbt/adapters/snowflake/impl.py +++ b/plugins/snowflake/dbt/adapters/snowflake/impl.py @@ -10,33 +10,30 @@ class SnowflakeAdapter(SQLAdapter): Relation = SnowflakeRelation ConnectionManager = SnowflakeConnectionManager - AdapterSpecificConfigs = frozenset({"transient"}) + AdapterSpecificConfigs = frozenset({"transient", "cluster_by", "automatic_clustering"}) @classmethod def date_function(cls): - return 'CURRENT_TIMESTAMP()' + return "CURRENT_TIMESTAMP()" @classmethod def _catalog_filter_table(cls, table, manifest): # On snowflake, users can set QUOTED_IDENTIFIERS_IGNORE_CASE, so force # the column names to their lowercased forms. - lowered = table.rename( - column_names=[c.lower() for c in table.column_names] - ) - return super(SnowflakeAdapter, cls)._catalog_filter_table(lowered, - manifest) + lowered = table.rename(column_names=[c.lower() for c in table.column_names]) + return super(SnowflakeAdapter, cls)._catalog_filter_table(lowered, manifest) def _make_match_kwargs(self, database, schema, identifier): quoting = self.config.quoting - if identifier is not None and quoting['identifier'] is False: + if identifier is not None and quoting["identifier"] is False: identifier = identifier.upper() - if schema is not None and quoting['schema'] is False: + if schema is not None and quoting["schema"] is False: schema = schema.upper() - if database is not None and quoting['database'] is False: + if database is not None and quoting["database"] is False: database = database.upper() - return filter_null_values({'identifier': identifier, - 'schema': schema, - 'database': database}) + return filter_null_values( + {"identifier": identifier, "schema": schema, "database": database} + ) diff --git a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql index 6b8a6bfdb12..e8c3ce49d7c 100644 --- a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql +++ b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql @@ -1,6 +1,7 @@ {% macro snowflake__create_table_as(temporary, relation, sql) -%} {%- set transient = config.get('transient', default=true) -%} - {%- set cluster_by_keys = config.get('cluster_by') -%} + {%- set cluster_by_keys = config.get('cluster_by', default=none) -%} + {%- set enable_automatic_clustering = config.get('automatic_clustering', default=false) -%} {%- set is_incremental = config.get('materialized') == 'incremental' -%} {%- if cluster_by_keys is not none and cluster_by_keys is string -%} {%- set cluster_by_keys = [cluster_by_keys] -%} @@ -21,10 +22,13 @@ {{ sql }} {%- endif %} ); - {% if cluster_by_keys is not none and is_incremental -%} + {% if cluster_by_keys is not none -%} alter table {{relation}} cluster by ({{cluster_by_string}}); + {%- endif -%} + {% if enable_automatic_clustering -%} alter table {{relation}} resume recluster; - {%- endif -%} + {%- endif -%} + From 954fed04d4522a88270665d72c1a35bc81415087 Mon Sep 17 00:00:00 2001 From: Bastien Boutonnet Date: Sat, 20 Jul 2019 19:24:03 +0200 Subject: [PATCH 10/11] remove is_incremental boolean check and remove useless witespace --- plugins/snowflake/dbt/include/snowflake/macros/adapters.sql | 4 ---- 1 file changed, 4 deletions(-) diff --git a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql index e8c3ce49d7c..a344993625d 100644 --- a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql +++ b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql @@ -2,7 +2,6 @@ {%- set transient = config.get('transient', default=true) -%} {%- set cluster_by_keys = config.get('cluster_by', default=none) -%} {%- set enable_automatic_clustering = config.get('automatic_clustering', default=false) -%} - {%- set is_incremental = config.get('materialized') == 'incremental' -%} {%- if cluster_by_keys is not none and cluster_by_keys is string -%} {%- set cluster_by_keys = [cluster_by_keys] -%} {%- set cluster_by_string = cluster_by_keys|join(", ")-%} @@ -29,9 +28,6 @@ alter table {{relation}} resume recluster; {%- endif -%} - - - {% endmacro %} {% macro snowflake__create_view_as(relation, sql) -%} From ba04a874234807f4c6ac8abd6afa18bdf65c477e Mon Sep 17 00:00:00 2001 From: Bastien Boutonnet Date: Wed, 31 Jul 2019 23:09:26 +0200 Subject: [PATCH 11/11] make max line length <= 79 chars --- plugins/snowflake/dbt/adapters/snowflake/impl.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/plugins/snowflake/dbt/adapters/snowflake/impl.py b/plugins/snowflake/dbt/adapters/snowflake/impl.py index eda01763368..5df44254b1e 100644 --- a/plugins/snowflake/dbt/adapters/snowflake/impl.py +++ b/plugins/snowflake/dbt/adapters/snowflake/impl.py @@ -10,7 +10,9 @@ class SnowflakeAdapter(SQLAdapter): Relation = SnowflakeRelation ConnectionManager = SnowflakeConnectionManager - AdapterSpecificConfigs = frozenset({"transient", "cluster_by", "automatic_clustering"}) + AdapterSpecificConfigs = frozenset( + {"transient", "cluster_by", "automatic_clustering"} + ) @classmethod def date_function(cls): @@ -20,8 +22,12 @@ def date_function(cls): def _catalog_filter_table(cls, table, manifest): # On snowflake, users can set QUOTED_IDENTIFIERS_IGNORE_CASE, so force # the column names to their lowercased forms. - lowered = table.rename(column_names=[c.lower() for c in table.column_names]) - return super(SnowflakeAdapter, cls)._catalog_filter_table(lowered, manifest) + lowered = table.rename( + column_names=[c.lower() for c in table.column_names] + ) + return super(SnowflakeAdapter, cls)._catalog_filter_table( + lowered, manifest + ) def _make_match_kwargs(self, database, schema, identifier): quoting = self.config.quoting