From 82ee51656dc4bd98acacba874b94ab6a056cf1bc Mon Sep 17 00:00:00 2001 From: Juri Krainjukov Date: Wed, 9 Aug 2023 20:30:54 +0000 Subject: [PATCH 01/15] persist view column comments --- dbt/include/spark/macros/adapters.sql | 29 +++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/dbt/include/spark/macros/adapters.sql b/dbt/include/spark/macros/adapters.sql index 471d1deef..4fb5884ce 100644 --- a/dbt/include/spark/macros/adapters.sql +++ b/dbt/include/spark/macros/adapters.sql @@ -229,9 +229,38 @@ {% endfor %} {% endmacro %} +{% macro get_column_comment_sql(column_name, column_dict) -%} + {% if (column_name|upper in column_dict) -%} + {% set matched_column = column_name|upper -%} + {% elif (column_name|lower in column_dict) -%} + {% set matched_column = column_name|lower -%} + {% elif (column_name in column_dict) -%} + {% set matched_column = column_name -%} + {% else -%} + {% set matched_column = None -%} + {% endif -%} + {% if matched_column and column_dict[matched_column]["description"] -%} + {% set column_comment_clause = "comment '" ~ column_dict[matched_column]["description"] ~ "'" %} + {%- endif -%} + {{ adapter.quote(column_name) }} {{ column_comment_clause }} +{% endmacro %} + +{% macro get_persist_docs_column_list(model_columns, query_columns) %} +( + {% for column_name in query_columns %} + {{ get_column_comment_sql(column_name, model_columns) }} + {{- ", " if not loop.last else "" }} + {% endfor %} +) +{% endmacro %} {% macro spark__create_view_as(relation, sql) -%} create or replace view {{ relation }} + {% if config.persist_column_docs() -%} + {% set model_columns = model.columns %} + {% set query_columns = get_columns_in_query(sql) %} + {{ get_persist_docs_column_list(model_columns, query_columns) }} + {% endif %} {{ comment_clause() }} {%- set contract_config = config.get('contract') -%} {%- if contract_config.enforced -%} From 2bc491403ec7e9f3f77d42a3c505eaec5281f766 Mon Sep 17 00:00:00 2001 From: Juri Krainjukov Date: Mon, 14 Aug 2023 09:12:37 +0000 Subject: [PATCH 02/15] format: whitespace --- dbt/include/spark/macros/adapters.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbt/include/spark/macros/adapters.sql b/dbt/include/spark/macros/adapters.sql index 4fb5884ce..960913d08 100644 --- a/dbt/include/spark/macros/adapters.sql +++ b/dbt/include/spark/macros/adapters.sql @@ -257,7 +257,7 @@ {% macro spark__create_view_as(relation, sql) -%} create or replace view {{ relation }} {% if config.persist_column_docs() -%} - {% set model_columns = model.columns %} + {% set model_columns = model.columns %} {% set query_columns = get_columns_in_query(sql) %} {{ get_persist_docs_column_list(model_columns, query_columns) }} {% endif %} From 2b8fef673e44f15a8d453928c5700ceced48aeb1 Mon Sep 17 00:00:00 2001 From: Juri Krainjukov Date: Thu, 17 Aug 2023 12:08:59 +0000 Subject: [PATCH 03/15] extracted get_matched_column macro --- dbt/include/spark/macros/adapters.sql | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dbt/include/spark/macros/adapters.sql b/dbt/include/spark/macros/adapters.sql index 960913d08..07502abb1 100644 --- a/dbt/include/spark/macros/adapters.sql +++ b/dbt/include/spark/macros/adapters.sql @@ -229,7 +229,7 @@ {% endfor %} {% endmacro %} -{% macro get_column_comment_sql(column_name, column_dict) -%} +{% macro get_matched_column(column_name, column_dict) %} {% if (column_name|upper in column_dict) -%} {% set matched_column = column_name|upper -%} {% elif (column_name|lower in column_dict) -%} @@ -239,6 +239,11 @@ {% else -%} {% set matched_column = None -%} {% endif -%} + {{ return(matched_column)}} +{% endmacro %} + +{% macro get_column_comment_sql(column_name, column_dict) -%} + {% set matched_column = get_matched_column(column_name, column_dict) -%} {% if matched_column and column_dict[matched_column]["description"] -%} {% set column_comment_clause = "comment '" ~ column_dict[matched_column]["description"] ~ "'" %} {%- endif -%} From a15cada7de0b4a756d193d50b2cce07256d24202 Mon Sep 17 00:00:00 2001 From: Juri Krainjukov Date: Thu, 17 Aug 2023 12:25:33 +0000 Subject: [PATCH 04/15] move parenthesis to the calling macro --- dbt/include/spark/macros/adapters.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbt/include/spark/macros/adapters.sql b/dbt/include/spark/macros/adapters.sql index 07502abb1..9b73696f1 100644 --- a/dbt/include/spark/macros/adapters.sql +++ b/dbt/include/spark/macros/adapters.sql @@ -251,12 +251,10 @@ {% endmacro %} {% macro get_persist_docs_column_list(model_columns, query_columns) %} -( {% for column_name in query_columns %} {{ get_column_comment_sql(column_name, model_columns) }} {{- ", " if not loop.last else "" }} {% endfor %} -) {% endmacro %} {% macro spark__create_view_as(relation, sql) -%} @@ -264,7 +262,9 @@ {% if config.persist_column_docs() -%} {% set model_columns = model.columns %} {% set query_columns = get_columns_in_query(sql) %} + ( {{ get_persist_docs_column_list(model_columns, query_columns) }} + ) {% endif %} {{ comment_clause() }} {%- set contract_config = config.get('contract') -%} From 9c5138f1827c18a60c2bd052339cc660311ce0e9 Mon Sep 17 00:00:00 2001 From: Juri Krainjukov Date: Thu, 17 Aug 2023 13:08:52 +0000 Subject: [PATCH 05/15] changelog --- .changes/unreleased/Features-20230817-130731.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Features-20230817-130731.yaml diff --git a/.changes/unreleased/Features-20230817-130731.yaml b/.changes/unreleased/Features-20230817-130731.yaml new file mode 100644 index 000000000..3818abef9 --- /dev/null +++ b/.changes/unreleased/Features-20230817-130731.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Persist Column level comments when creating views +time: 2023-08-17T13:07:31.6812862Z +custom: + Author: jurasan + Issue: CT-764 From 50e602eddd1f4d1f7fff46891b23e087a266c633 Mon Sep 17 00:00:00 2001 From: Juri Krainjukov Date: Fri, 8 Sep 2023 16:32:51 +0000 Subject: [PATCH 06/15] fix: remove matching column in different case --- dbt/include/spark/macros/adapters.sql | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/dbt/include/spark/macros/adapters.sql b/dbt/include/spark/macros/adapters.sql index 9b73696f1..8b1d15c36 100644 --- a/dbt/include/spark/macros/adapters.sql +++ b/dbt/include/spark/macros/adapters.sql @@ -230,11 +230,7 @@ {% endmacro %} {% macro get_matched_column(column_name, column_dict) %} - {% if (column_name|upper in column_dict) -%} - {% set matched_column = column_name|upper -%} - {% elif (column_name|lower in column_dict) -%} - {% set matched_column = column_name|lower -%} - {% elif (column_name in column_dict) -%} + {% if (column_name in column_dict) -%} {% set matched_column = column_name -%} {% else -%} {% set matched_column = None -%} From 1ea41786b6e922e504abb32352df9bffea101f15 Mon Sep 17 00:00:00 2001 From: Juri Krainjukov Date: Fri, 8 Sep 2023 17:24:04 +0000 Subject: [PATCH 07/15] fix: remove get_matched_column macro - not much logic left there. --- dbt/include/spark/macros/adapters.sql | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/dbt/include/spark/macros/adapters.sql b/dbt/include/spark/macros/adapters.sql index 8b1d15c36..6b2303657 100644 --- a/dbt/include/spark/macros/adapters.sql +++ b/dbt/include/spark/macros/adapters.sql @@ -229,19 +229,9 @@ {% endfor %} {% endmacro %} -{% macro get_matched_column(column_name, column_dict) %} - {% if (column_name in column_dict) -%} - {% set matched_column = column_name -%} - {% else -%} - {% set matched_column = None -%} - {% endif -%} - {{ return(matched_column)}} -{% endmacro %} - {% macro get_column_comment_sql(column_name, column_dict) -%} - {% set matched_column = get_matched_column(column_name, column_dict) -%} - {% if matched_column and column_dict[matched_column]["description"] -%} - {% set column_comment_clause = "comment '" ~ column_dict[matched_column]["description"] ~ "'" %} + {% if column_name in column_dict and column_dict[column_name]["description"] -%} + {% set column_comment_clause = "comment '" ~ column_dict[column_name]["description"] ~ "'" %} {%- endif -%} {{ adapter.quote(column_name) }} {{ column_comment_clause }} {% endmacro %} From af3dbaa22a0595e5b185451fbc911d0028bf931a Mon Sep 17 00:00:00 2001 From: Colin Date: Mon, 25 Sep 2023 11:41:52 -0700 Subject: [PATCH 08/15] escape column comments and add functional test --- dbt/include/spark/macros/adapters.sql | 3 +- .../adapter/persist_docs/fixtures.py | 5 +++ .../adapter/persist_docs/test_persist_docs.py | 43 ++++++++++++++++++- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/dbt/include/spark/macros/adapters.sql b/dbt/include/spark/macros/adapters.sql index 6b2303657..9e277dd68 100644 --- a/dbt/include/spark/macros/adapters.sql +++ b/dbt/include/spark/macros/adapters.sql @@ -231,7 +231,8 @@ {% macro get_column_comment_sql(column_name, column_dict) -%} {% if column_name in column_dict and column_dict[column_name]["description"] -%} - {% set column_comment_clause = "comment '" ~ column_dict[column_name]["description"] ~ "'" %} + {% set escaped_description = column_dict[column_name]["description"] | replace("'", "\\'") %} + {% set column_comment_clause = "comment '" ~ escaped_description ~ "'" %} {%- endif -%} {{ adapter.quote(column_name) }} {{ column_comment_clause }} {% endmacro %} diff --git a/tests/functional/adapter/persist_docs/fixtures.py b/tests/functional/adapter/persist_docs/fixtures.py index 3c351ab55..b3433c475 100644 --- a/tests/functional/adapter/persist_docs/fixtures.py +++ b/tests/functional/adapter/persist_docs/fixtures.py @@ -21,6 +21,11 @@ select 1 as id, 'Joe' as name """ +_MODELS__VIEW_DELTA_MODEL = """ +{{ config(materialized='view') }} +select id, count(*) as count from {{ ref('table_delta_model') }} +""" + _MODELS__TABLE_DELTA_MODEL_MISSING_COLUMN = """ {{ config(materialized='table', file_format='delta') }} select 1 as id, 'Joe' as different_name diff --git a/tests/functional/adapter/persist_docs/test_persist_docs.py b/tests/functional/adapter/persist_docs/test_persist_docs.py index 0e3d102dc..b6ae8043b 100644 --- a/tests/functional/adapter/persist_docs/test_persist_docs.py +++ b/tests/functional/adapter/persist_docs/test_persist_docs.py @@ -9,7 +9,7 @@ _MODELS__TABLE_DELTA_MODEL_MISSING_COLUMN, _PROPERTIES__MODELS, _PROPERTIES__SEEDS, - _SEEDS__BASIC, + _SEEDS__BASIC, _MODELS__VIEW_DELTA_MODEL, ) @@ -76,6 +76,47 @@ def test_delta_comments(self, project): assert result[2].startswith("Some stuff here and then a call to") +@pytest.mark.skip_profile("apache_spark", "spark_session") +class TestPersistDocsDeltaView: + @pytest.fixture(scope="class") + def models(self): + return { + "view_delta_model.sql": _MODELS__VIEW_DELTA_MODEL, + "schema.yml": _PROPERTIES__MODELS, + } + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "models": { + "test": { + "+persist_docs": { + "relation": True, + "columns": True, + }, + } + }, + } + + def test_delta_comments(self, project): + run_dbt(["run"]) + + results = project.run_sql( + "describe extended {schema}.{table}".format( + schema=project.test_schema, table="view_delta_model" + ), + fetch="all", + ) + + for result in results: + if result[0] == "Comment": + assert result[1].startswith(f"View model description") + if result[0] == "id": + assert result[2].startswith("id Column description") + if result[0] == "count": + self.assertEqual(result[2], "") + + @pytest.mark.skip_profile("apache_spark", "spark_session") class TestPersistDocsMissingColumn: @pytest.fixture(scope="class") From 58d60402455d39e105a9334f46f9f099d5d1c3c5 Mon Sep 17 00:00:00 2001 From: colin-rogers-dbt <111200756+colin-rogers-dbt@users.noreply.github.com> Date: Mon, 25 Sep 2023 11:46:06 -0700 Subject: [PATCH 09/15] Update Features-20230817-130731.yaml --- .changes/unreleased/Features-20230817-130731.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/unreleased/Features-20230817-130731.yaml b/.changes/unreleased/Features-20230817-130731.yaml index 3818abef9..e88deb7bd 100644 --- a/.changes/unreleased/Features-20230817-130731.yaml +++ b/.changes/unreleased/Features-20230817-130731.yaml @@ -3,4 +3,4 @@ body: Persist Column level comments when creating views time: 2023-08-17T13:07:31.6812862Z custom: Author: jurasan - Issue: CT-764 + Issue: 372 From 52b8326bf9c6ecee085c91036c281963af16c24e Mon Sep 17 00:00:00 2001 From: Colin Date: Mon, 25 Sep 2023 11:54:00 -0700 Subject: [PATCH 10/15] remove unneeded f string --- tests/functional/adapter/persist_docs/test_persist_docs.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/functional/adapter/persist_docs/test_persist_docs.py b/tests/functional/adapter/persist_docs/test_persist_docs.py index b6ae8043b..dfcc277fc 100644 --- a/tests/functional/adapter/persist_docs/test_persist_docs.py +++ b/tests/functional/adapter/persist_docs/test_persist_docs.py @@ -9,7 +9,8 @@ _MODELS__TABLE_DELTA_MODEL_MISSING_COLUMN, _PROPERTIES__MODELS, _PROPERTIES__SEEDS, - _SEEDS__BASIC, _MODELS__VIEW_DELTA_MODEL, + _SEEDS__BASIC, + _MODELS__VIEW_DELTA_MODEL, ) @@ -110,7 +111,7 @@ def test_delta_comments(self, project): for result in results: if result[0] == "Comment": - assert result[1].startswith(f"View model description") + assert result[1].startswith("View model description") if result[0] == "id": assert result[2].startswith("id Column description") if result[0] == "count": From b2b0c40bf0054cdefa369c00b186b72e7b37f442 Mon Sep 17 00:00:00 2001 From: Colin Date: Mon, 25 Sep 2023 12:56:11 -0700 Subject: [PATCH 11/15] add test fixture to view test --- tests/functional/adapter/persist_docs/test_persist_docs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/adapter/persist_docs/test_persist_docs.py b/tests/functional/adapter/persist_docs/test_persist_docs.py index dfcc277fc..edfcd6b7f 100644 --- a/tests/functional/adapter/persist_docs/test_persist_docs.py +++ b/tests/functional/adapter/persist_docs/test_persist_docs.py @@ -82,6 +82,7 @@ class TestPersistDocsDeltaView: @pytest.fixture(scope="class") def models(self): return { + "table_delta_model.sql": _MODELS__TABLE_DELTA_MODEL, "view_delta_model.sql": _MODELS__VIEW_DELTA_MODEL, "schema.yml": _PROPERTIES__MODELS, } From fff15ab12b898b2676ee895916ebf7d5510aa421 Mon Sep 17 00:00:00 2001 From: Colin Date: Mon, 25 Sep 2023 14:52:41 -0700 Subject: [PATCH 12/15] fix fixtures for TestPersistDocsDeltaView --- .../adapter/persist_docs/fixtures.py | 23 +++++++++++++++++++ .../adapter/persist_docs/test_persist_docs.py | 4 ++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/tests/functional/adapter/persist_docs/fixtures.py b/tests/functional/adapter/persist_docs/fixtures.py index b3433c475..deca02ba6 100644 --- a/tests/functional/adapter/persist_docs/fixtures.py +++ b/tests/functional/adapter/persist_docs/fixtures.py @@ -30,7 +30,30 @@ {{ config(materialized='table', file_format='delta') }} select 1 as id, 'Joe' as different_name """ +_VIEW_PROPERTIES_MODELS = """ +version: 2 +models: + - name: view_delta_model + description: | + Incremental model description "with double quotes" + and with 'single quotes' as welll as other; + '''abc123''' + reserved -- characters + -- + /* comment */ + Some $lbl$ labeled $lbl$ and $$ unlabeled $$ dollar-quoting + columns: + - name: id + description: | + id Column description "with double quotes" + and with 'single quotes' as welll as other; + '''abc123''' + reserved -- characters + -- + /* comment */ + Some $lbl$ labeled $lbl$ and $$ unlabeled $$ dollar-quoting +""" _PROPERTIES__MODELS = """ version: 2 diff --git a/tests/functional/adapter/persist_docs/test_persist_docs.py b/tests/functional/adapter/persist_docs/test_persist_docs.py index edfcd6b7f..e6dac7b47 100644 --- a/tests/functional/adapter/persist_docs/test_persist_docs.py +++ b/tests/functional/adapter/persist_docs/test_persist_docs.py @@ -10,7 +10,7 @@ _PROPERTIES__MODELS, _PROPERTIES__SEEDS, _SEEDS__BASIC, - _MODELS__VIEW_DELTA_MODEL, + _MODELS__VIEW_DELTA_MODEL, _VIEW_PROPERTIES_MODELS, ) @@ -84,7 +84,7 @@ def models(self): return { "table_delta_model.sql": _MODELS__TABLE_DELTA_MODEL, "view_delta_model.sql": _MODELS__VIEW_DELTA_MODEL, - "schema.yml": _PROPERTIES__MODELS, + "schema.yml": _VIEW_PROPERTIES_MODELS, } @pytest.fixture(scope="class") From c0dc04f73137a4a2ce0e7ee263b345a1c2fc4810 Mon Sep 17 00:00:00 2001 From: Colin Date: Mon, 25 Sep 2023 15:29:32 -0700 Subject: [PATCH 13/15] fix fixtures for TestPersistDocsDeltaView --- tests/functional/adapter/persist_docs/fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/adapter/persist_docs/fixtures.py b/tests/functional/adapter/persist_docs/fixtures.py index deca02ba6..8865bfdcb 100644 --- a/tests/functional/adapter/persist_docs/fixtures.py +++ b/tests/functional/adapter/persist_docs/fixtures.py @@ -23,7 +23,7 @@ _MODELS__VIEW_DELTA_MODEL = """ {{ config(materialized='view') }} -select id, count(*) as count from {{ ref('table_delta_model') }} +select id, count(*) as count from {{ ref('table_delta_model') }} group by id """ _MODELS__TABLE_DELTA_MODEL_MISSING_COLUMN = """ From 215fda49574e678076c29158702074a9c765f20e Mon Sep 17 00:00:00 2001 From: Colin Date: Mon, 25 Sep 2023 15:35:35 -0700 Subject: [PATCH 14/15] formatting --- tests/functional/adapter/persist_docs/test_persist_docs.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/functional/adapter/persist_docs/test_persist_docs.py b/tests/functional/adapter/persist_docs/test_persist_docs.py index e6dac7b47..e72013877 100644 --- a/tests/functional/adapter/persist_docs/test_persist_docs.py +++ b/tests/functional/adapter/persist_docs/test_persist_docs.py @@ -10,7 +10,8 @@ _PROPERTIES__MODELS, _PROPERTIES__SEEDS, _SEEDS__BASIC, - _MODELS__VIEW_DELTA_MODEL, _VIEW_PROPERTIES_MODELS, + _MODELS__VIEW_DELTA_MODEL, + _VIEW_PROPERTIES_MODELS, ) From a4f0b0cc361c0237a2223ac08509e0904fbaf3ba Mon Sep 17 00:00:00 2001 From: Colin Date: Mon, 25 Sep 2023 16:08:18 -0700 Subject: [PATCH 15/15] fix tests --- tests/functional/adapter/persist_docs/fixtures.py | 2 +- tests/functional/adapter/persist_docs/test_persist_docs.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/adapter/persist_docs/fixtures.py b/tests/functional/adapter/persist_docs/fixtures.py index 8865bfdcb..b884b7dec 100644 --- a/tests/functional/adapter/persist_docs/fixtures.py +++ b/tests/functional/adapter/persist_docs/fixtures.py @@ -36,7 +36,7 @@ models: - name: view_delta_model description: | - Incremental model description "with double quotes" + View model description "with double quotes" and with 'single quotes' as welll as other; '''abc123''' reserved -- characters diff --git a/tests/functional/adapter/persist_docs/test_persist_docs.py b/tests/functional/adapter/persist_docs/test_persist_docs.py index e72013877..ee02e5ef8 100644 --- a/tests/functional/adapter/persist_docs/test_persist_docs.py +++ b/tests/functional/adapter/persist_docs/test_persist_docs.py @@ -117,7 +117,7 @@ def test_delta_comments(self, project): if result[0] == "id": assert result[2].startswith("id Column description") if result[0] == "count": - self.assertEqual(result[2], "") + assert result[2] is None @pytest.mark.skip_profile("apache_spark", "spark_session")