Merge pull request #2321 from snowflakeseitz/column_comments

Column comments
dbt-labs · May 5, 2020 · 7c916e9 · 7c916e9
2 parents 9de9335 + b774702
commit 7c916e9
Show file tree

Hide file tree

Showing 8 changed files with 113 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,6 +29,8 @@
 - Sources from dependencies can be overridden in schema.yml files ([#2287](https:/fishtown-analytics/dbt/issues/2287), [#2357](https:/fishtown-analytics/dbt/pull/2357))
 - Implement persist_docs for both `relation` and `comments` on postgres and redshift, and extract them when getting the catalog. ([#2333](https:/fishtown-analytics/dbt/issues/2333), [#2378](https:/fishtown-analytics/dbt/pull/2378))
 - Added a filter named `as_text` to the native environment rendering code that allows users to mark a value as always being a string ([#2384](https:/fishtown-analytics/dbt/issues/2384), [#2395](https:/fishtown-analytics/dbt/pull/2395))
+- Relation comments supported for Snowflake tables and views. Column comments supported for tables. ([#1722](https:/fishtown-analytics/dbt/issues/1722), [#2321](https:/fishtown-analytics/dbt/pull/2321))
+
 
 ### Fixes
 - When a jinja value is undefined, give a helpful error instead of failing with cryptic "cannot pickle ParserMacroCapture" errors ([#2110](https:/fishtown-analytics/dbt/issues/2110), [#2184](https:/fishtown-analytics/dbt/pull/2184))
@@ -69,6 +71,7 @@ Contributors:
  - [@Fokko](https:/Fokko) [#2361](https:/fishtown-analytics/dbt/pull/2361)
  - [@franloza](https:/franloza) [#2349](https:/fishtown-analytics/dbt/pull/2349)
  - [@sethwoodworth](https:/sethwoodworth) [#2389](https:/fishtown-analytics/dbt/pull/2389)
+ - [@snowflakeseitz](https:/snowflakeseitz) [#2321](https:/fishtown-analytics/dbt/pull/2321)
 
 ## dbt 0.16.1 (April 14, 2020)
 
@@ -177,7 +180,7 @@ Contributors:
 
 ### Features
 - Add column-level quoting control for tests ([#2106](https:/fishtown-analytics/dbt/issues/2106), [#2047](https:/fishtown-analytics/dbt/pull/2047))
-- Add the macros every node uses to its `depends_on.macros` list ([#2082](https:/fishtown-analytics/dbt/issues/2082), [#2103](https:/fishtown-analytics/dbt/pull/2103))
+- Add the macros every node uses to its `depends_on.macros` list ([#2082](https:/fishtown-analytics/dbt/issues/2082), [#2103](https:/fishtown-analytics/dbt/pull/2103))
 - Add `arguments` field to macros ([#2081](https:/fishtown-analytics/dbt/issues/2081), [#2083](https:/fishtown-analytics/dbt/issues/2083), [#2096](https:/fishtown-analytics/dbt/pull/2096))
 - Batch the anonymous usage statistics requests to improve performance ([#2008](https:/fishtown-analytics/dbt/issues/2008), [#2089](https:/fishtown-analytics/dbt/pull/2089))
 - Add documentation for macros/analyses ([#1041](https:/fishtown-analytics/dbt/issues/1041), [#2068](https:/fishtown-analytics/dbt/pull/2068))
@@ -2227,4 +2230,4 @@ profile: "side-project"
 
 model-defaults:
 ....
-```
+```
diff --git a/core/dbt/include/global_project/macros/adapters/common.sql b/core/dbt/include/global_project/macros/adapters/common.sql
@@ -131,6 +131,29 @@
  {{ return(adapter_macro('alter_column_type', relation, column_name, new_column_type)) }}
 {% endmacro %}
 
+
+
+{% macro alter_column_comment(relation, column_dict) -%}
+ {{ return(adapter_macro('alter_column_comment', relation, column_dict)) }}
+{% endmacro %}
+
+{% macro default__alter_column_comment(relation, column_dict) -%}
+ {{ exceptions.raise_not_implemented(
+ 'alter_column_comment macro not implemented for adapter '+adapter.type()) }}
+{% endmacro %}
+
+{% macro alter_relation_comment(relation, relation_comment) -%}
+ {{ return(adapter_macro('alter_relation_comment', relation, relation_comment)) }}
+{% endmacro %}
+
+{% macro default__alter_relation_comment(relation, relation_comment) -%}
+ {{ exceptions.raise_not_implemented(
+ 'alter_relation_comment macro not implemented for adapter '+adapter.type()) }}
+{% endmacro %}
+
+
+
+
 {% macro default__alter_column_type(relation, column_name, new_column_type) -%}
  {#
  1. Create a new column (w/ temp name and correct type)
@@ -300,22 +323,3 @@
  {%- endif -%}
 {%- endmacro -%}
 
-
-{# copy+pasted from the snowflake PR - delete these 4 on merge #}
-{% macro alter_column_comment(relation, column_dict) -%}
- {{ return(adapter_macro('alter_column_comment', relation, column_dict)) }}
-{% endmacro %}
-
-{% macro default__alter_column_comment(relation, column_dict) -%}
- {{ exceptions.raise_not_implemented(
- 'alter_column_comment macro not implemented for adapter '+adapter.type()) }}
-{% endmacro %}
-
-{% macro alter_relation_comment(relation, relation_comment) -%}
- {{ return(adapter_macro('alter_relation_comment', relation, relation_comment)) }}
-{% endmacro %}
-
-{% macro default__alter_relation_comment(relation, relation_comment) -%}
- {{ exceptions.raise_not_implemented(
- 'alter_relation_comment macro not implemented for adapter '+adapter.type()) }}
-{% endmacro %}
diff --git a/core/dbt/include/global_project/macros/etc/get_relation_comment.sql b/core/dbt/include/global_project/macros/etc/get_relation_comment.sql
@@ -1,7 +1,7 @@
 {% macro get_relation_comment(persist_docs, model) %}
 
  {%- if persist_docs is not mapping -%}
- {{ exceptions.raise_compiler_error("Invalid value provided for 'persist_docs'. Expected dict but got value: " ~ raw_persist_docs) }}
+ {{ exceptions.raise_compiler_error("Invalid value provided for 'persist_docs'. Expected dict but got value: " ~ persist_docs) }}
  {% endif %}
 
  {% if persist_docs.get('relation', false) %}
@@ -13,8 +13,8 @@
 {% endmacro %}
 
 
-{# copy+pasted from the snowflake PR - delete this on merge #}
 {% macro get_relation_column_comments(persist_docs, model) %}
+
  {%- if persist_docs is not mapping -%}
  {{ exceptions.raise_compiler_error("Invalid value provided for 'persist_docs'. Expected dict but got value: " ~ persist_docs) }}
  {% endif %}
@@ -26,3 +26,4 @@
  {% endif %}
 
 {% endmacro %}
+
diff --git a/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql b/plugins/snowflake/dbt/include/snowflake/macros/adapters.sql
@@ -3,6 +3,10 @@
  {%- set cluster_by_keys = config.get('cluster_by', default=none) -%}
  {%- set enable_automatic_clustering = config.get('automatic_clustering', default=false) -%}
  {%- set copy_grants = config.get('copy_grants', default=false) -%}
+ {%- set raw_persist_docs = config.get('persist_docs', {}) -%}
+ {%- set relation_comment = get_relation_comment(raw_persist_docs, model) -%}
+ {%- set column_comment = get_relation_column_comments(raw_persist_docs, model) -%}
+
  {%- if cluster_by_keys is not none and cluster_by_keys is string -%}
  {%- set cluster_by_keys = [cluster_by_keys] -%}
  {%- endif -%}
@@ -35,20 +39,38 @@
  {% if enable_automatic_clustering and cluster_by_string is not none and not temporary -%}
  alter table {{relation}} resume recluster;
  {%- endif -%}
+ -- add in comments
+
+ {% set relation = relation.incorporate(type='table') %}
+ {% if relation_comment is not none -%}
+ {{ alter_relation_comment(relation, relation_comment) }}
+ {%- endif -%}
+
+ {% if column_comment is not none -%}
+ {{ alter_column_comment(relation, column_comment) }}
+ {%- endif -%}
 
 {% endmacro %}
 
 {% macro snowflake__create_view_as(relation, sql) -%}
  {%- set secure = config.get('secure', default=false) -%}
  {%- set copy_grants = config.get('copy_grants', default=false) -%}
  {%- set sql_header = config.get('sql_header', none) -%}
+ {%- set raw_persist_docs = config.get('persist_docs', {}) -%}
+ {%- set relation_comment = get_relation_comment(raw_persist_docs, model) -%}
 
  {{ sql_header if sql_header is not none }}
  create or replace {% if secure -%}
  secure
  {%- endif %} view {{ relation }} {% if copy_grants -%} copy grants {%- endif %} as (
  {{ sql }}
  );
+
+ {%- set relation = relation.incorporate(type='view') -%}
+ {% if relation_comment is not none -%}
+ {{ alter_relation_comment(relation, relation_comment) }}
+ {%- endif -%}
+
 {% endmacro %}
 
 {% macro snowflake__get_columns_in_relation(relation) -%}
@@ -150,3 +172,19 @@
  alter table {{ relation }} alter {{ adapter.quote(column_name) }} set data type {{ new_column_type }};
  {% endcall %}
 {% endmacro %}
+
+{% macro snowflake__alter_relation_comment(relation, relation_comment) -%}
+ comment on {{ relation.type }} {{ relation }} IS $${{ relation_comment | replace('$', '[$]') }}$$;
+{% endmacro %}
+
+
+{% macro snowflake__alter_column_comment(relation, column_dict) -%}
+ alter {{ relation.type }} {{ relation }} alter
+ {% for column_name in column_dict %}
+ {{ column_name }} COMMENT $${{ column_dict[column_name]['description'] | replace('$', '[$]') }}$$ {{ ',' if not loop.last else ';' }}
+ {% endfor %}
+{% endmacro %}
+
+
+
+
diff --git a/test/integration/059_persist_docs_test/sf_models/incremental_test.sql b/test/integration/059_persist_docs_test/sf_models/incremental_test.sql
@@ -0,0 +1,8 @@
+{{
+ config ({
+ "materialized" : 'incremental',
+ "persist_docs" : { "relation": true, "columns": true, "schema": true }
+ })
+}}
+
+select 1 as column1
diff --git a/test/integration/059_persist_docs_test/sf_models/schema.yml b/test/integration/059_persist_docs_test/sf_models/schema.yml
@@ -0,0 +1,21 @@
+models:
+ - name: view_test
+ description: >
+ View - "money" aka $$$$$
+ columns:
+ - name: column1
+ description: test 1 - abcd "" $$$ '' 
+
+ - name: incremental_test
+ description: >
+ Incremental - "money" aka $$$$$
+ columns:
+ - name: column1
+ description: test 1 - abcd "" $$$ '' 
+
+ - name: table_test
+ description: >
+ Table - "money" aka $$$$$
+ columns:
+ - name: column1
+ description: test 1 - abcd "" $$$ '' 
diff --git a/test/integration/059_persist_docs_test/sf_models/table_test.sql b/test/integration/059_persist_docs_test/sf_models/table_test.sql
@@ -0,0 +1,8 @@
+{{
+ config ({
+ "materialized" : 'table',
+ "persist_docs" : { "relation": true, "columns": true, "schema": true }
+ })
+}}
+
+select 1 as column1
diff --git a/test/integration/059_persist_docs_test/sf_models/view_test.sql b/test/integration/059_persist_docs_test/sf_models/view_test.sql
@@ -0,0 +1,7 @@
+{{
+ config ({
+ "persist_docs" : { "relation": true, "columns": true, "schema": true }
+ })
+}}
+
+select 1 as column1