From 053546402d0fdb56efc01e408bd7eda1e414f8b3 Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Tue, 12 May 2020 14:24:46 -0600 Subject: [PATCH] Only run alter_column_comment if there are columns defined Also remove unused (and confusing/misleading) test data --- CHANGELOG.md | 7 ++++++- .../global_project/macros/adapters/common.sql | 4 ++-- .../dbt/include/bigquery/macros/adapters.sql | 2 +- .../dbt/include/redshift/macros/adapters.sql | 4 ++-- .../sf_models/incremental_test.sql | 8 ------- .../sf_models/schema.yml | 21 ------------------- .../sf_models/table_test.sql | 8 ------- .../sf_models/view_test.sql | 7 ------- .../models/no_docs_model.sql | 1 + .../test_persist_docs.py | 13 +++++++----- 10 files changed, 20 insertions(+), 55 deletions(-) delete mode 100644 test/integration/059_persist_docs_test/sf_models/incremental_test.sql delete mode 100644 test/integration/059_persist_docs_test/sf_models/schema.yml delete mode 100644 test/integration/059_persist_docs_test/sf_models/table_test.sql delete mode 100644 test/integration/059_persist_docs_test/sf_models/view_test.sql create mode 100644 test/integration/060_persist_docs_tests/models/no_docs_model.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index a8c2036bfc7..c1961f735a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ ## dbt 0.17.0 (Release TBD) + +### Fixes +- When no columns are documented and persist_docs.columns is True, skip creating comments instead of failing with errors ([#2439](https://github.com/fishtown-analytics/dbt/issues/2439), [#2440](https://github.com/fishtown-analytics/dbt/pull/2440)) + + ## dbt 0.17.0rc1 (May 12, 2020) ### Breaking changes @@ -26,7 +31,7 @@ - Track distinct project hashes in anonymous usage metrics for package downloads ([#2351](https://github.com/fishtown-analytics/dbt/issues/2351), [#2429](https://github.com/fishtown-analytics/dbt/pull/2429)) Contributors: - - [@azhard](https://github.com/azhard) ([#2413](https://github.com/fishtown-analytics/dbt/pull/2413), [#2422](https://github.com/fishtown-analytics/dbt/pull/2422)) + - [@azhard](https://github.com/azhard) ([#2413](https://github.com/fishtown-analytics/dbt/pull/2413), [#2422](https://github.com/fishtown-analytics/dbt/pull/2422)) - [@mikaelene](https://github.com/mikaelene) [#2414](https://github.com/fishtown-analytics/dbt/pull/2414) - [@raalsky](https://github.com/Raalsky) ([#2343](https://github.com/fishtown-analytics/dbt/pull/2343)) - [@alf-mindshift](https://github.com/alf-mindshift) ([docs#90](https://github.com/fishtown-analytics/dbt-docs/pull/90)) diff --git a/core/dbt/include/global_project/macros/adapters/common.sql b/core/dbt/include/global_project/macros/adapters/common.sql index 23f548eb2cb..9dcc71003fe 100644 --- a/core/dbt/include/global_project/macros/adapters/common.sql +++ b/core/dbt/include/global_project/macros/adapters/common.sql @@ -156,11 +156,11 @@ {% endmacro %} {% macro default__persist_docs(relation, model, for_relation, for_columns) -%} - {% if for_relation and config.persist_relation_docs() %} + {% if for_relation and config.persist_relation_docs() and model.description %} {% do run_query(alter_relation_comment(relation, model.description)) %} {% endif %} - {% if for_columns and config.persist_column_docs() %} + {% if for_columns and config.persist_column_docs() and model.columns %} {% do run_query(alter_column_comment(relation, model.columns)) %} {% endif %} {% endmacro %} diff --git a/plugins/bigquery/dbt/include/bigquery/macros/adapters.sql b/plugins/bigquery/dbt/include/bigquery/macros/adapters.sql index ef4b39a0130..5bf24ef2df3 100644 --- a/plugins/bigquery/dbt/include/bigquery/macros/adapters.sql +++ b/plugins/bigquery/dbt/include/bigquery/macros/adapters.sql @@ -115,7 +115,7 @@ {#-- relation-level macro is not implemented. This is handled in the CTAs statement #} {% macro bigquery__persist_docs(relation, model, for_relation, for_columns) -%} - {% if for_columns and config.persist_column_docs() %} + {% if for_columns and config.persist_column_docs() and model.columns %} {% do alter_column_comment(relation, model.columns) %} {% endif %} {% endmacro %} diff --git a/plugins/redshift/dbt/include/redshift/macros/adapters.sql b/plugins/redshift/dbt/include/redshift/macros/adapters.sql index a6bc82c2b8e..2d05ce551e3 100644 --- a/plugins/redshift/dbt/include/redshift/macros/adapters.sql +++ b/plugins/redshift/dbt/include/redshift/macros/adapters.sql @@ -192,13 +192,13 @@ {% macro redshift__persist_docs(relation, model, for_relation, for_columns) -%} - {% if for_relation and config.persist_relation_docs() %} + {% if for_relation and config.persist_relation_docs() and model.description %} {% do run_query(alter_relation_comment(relation, model.description)) %} {% endif %} {# Override: do not set column comments for LBVs #} {% set is_lbv = config.get('materialized') == 'view' and config.get('bind') == false %} - {% if for_columns and config.persist_column_docs() and not is_lbv %} + {% if for_columns and config.persist_column_docs() and model.columns and not is_lbv %} {% do run_query(alter_column_comment(relation, model.columns)) %} {% endif %} {% 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 deleted file mode 100644 index a4b905a931a..00000000000 --- a/test/integration/059_persist_docs_test/sf_models/incremental_test.sql +++ /dev/null @@ -1,8 +0,0 @@ -{{ - 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 deleted file mode 100644 index ad1ab4daa23..00000000000 --- a/test/integration/059_persist_docs_test/sf_models/schema.yml +++ /dev/null @@ -1,21 +0,0 @@ -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 deleted file mode 100644 index 447f3489712..00000000000 --- a/test/integration/059_persist_docs_test/sf_models/table_test.sql +++ /dev/null @@ -1,8 +0,0 @@ -{{ - 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 deleted file mode 100644 index 337d4ccc098..00000000000 --- a/test/integration/059_persist_docs_test/sf_models/view_test.sql +++ /dev/null @@ -1,7 +0,0 @@ -{{ - config ({ - "persist_docs" : { "relation": true, "columns": true, "schema": true } - }) -}} - -select 1 as column1 diff --git a/test/integration/060_persist_docs_tests/models/no_docs_model.sql b/test/integration/060_persist_docs_tests/models/no_docs_model.sql new file mode 100644 index 00000000000..e39a7a1566f --- /dev/null +++ b/test/integration/060_persist_docs_tests/models/no_docs_model.sql @@ -0,0 +1 @@ +select 1 as id, 'Alice' as name diff --git a/test/integration/060_persist_docs_tests/test_persist_docs.py b/test/integration/060_persist_docs_tests/test_persist_docs.py index 3dd9f0da140..94bfe5715f3 100644 --- a/test/integration/060_persist_docs_tests/test_persist_docs.py +++ b/test/integration/060_persist_docs_tests/test_persist_docs.py @@ -75,13 +75,16 @@ def run_has_comments_pglike(self): with open('target/catalog.json') as fp: catalog_data = json.load(fp) assert 'nodes' in catalog_data - assert len(catalog_data['nodes']) == 2 + assert len(catalog_data['nodes']) == 3 table_node = catalog_data['nodes']['model.test.table_model'] view_node = self._assert_has_table_comments(table_node) view_node = catalog_data['nodes']['model.test.view_model'] self._assert_has_view_comments(view_node) + no_docs_node = catalog_data['nodes']['model.test.no_docs_model'] + self._assert_has_view_comments(no_docs_node, False, False) + @use_profile('postgres') def test_postgres_comments(self): self.run_has_comments_pglike() @@ -116,13 +119,16 @@ def test_redshift_late_binding_view(self): with open('target/catalog.json') as fp: catalog_data = json.load(fp) assert 'nodes' in catalog_data - assert len(catalog_data['nodes']) == 2 + assert len(catalog_data['nodes']) == 3 table_node = catalog_data['nodes']['model.test.table_model'] view_node = self._assert_has_table_comments(table_node) view_node = catalog_data['nodes']['model.test.view_model'] self._assert_has_view_comments(view_node, False, False) + no_docs_node = catalog_data['nodes']['model.test.no_docs_model'] + self._assert_has_view_comments(no_docs_node, False, False) + class TestPersistDocsSimple(BasePersistDocsTest): @property @@ -135,9 +141,6 @@ def project_config(self): "relation": True, "columns": True, }, - 'view_model': { - 'bind': False, - } } } }