Merge pull request #2442 from fishtown-analytics/fix/empty-persist-docs

Backport #2440 to 0.17
dbt-labs · May 13, 2020 · 1de892b · 1de892b
2 parents 79a90d0 + 0535464
commit 1de892b
Show file tree

Hide file tree

Showing 10 changed files with 20 additions and 55 deletions.
diff --git 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:/fishtown-analytics/dbt/issues/2439), [#2440](https:/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:/fishtown-analytics/dbt/issues/2351), [#2429](https:/fishtown-analytics/dbt/pull/2429))
 
 Contributors:
- - [@azhard](https:/azhard) ([#2413](https:/fishtown-analytics/dbt/pull/2413), [#2422](https:/fishtown-analytics/dbt/pull/2422)) 
+ - [@azhard](https:/azhard) ([#2413](https:/fishtown-analytics/dbt/pull/2413), [#2422](https:/fishtown-analytics/dbt/pull/2422))
  - [@mikaelene](https:/mikaelene) [#2414](https:/fishtown-analytics/dbt/pull/2414)
  - [@raalsky](https:/Raalsky) ([#2343](https:/fishtown-analytics/dbt/pull/2343))
  - [@alf-mindshift](https:/alf-mindshift) ([docs#90](https:/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
@@ -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
@@ -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
@@ -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
diff --git a/test/integration/059_persist_docs_test/sf_models/schema.yml b/test/integration/059_persist_docs_test/sf_models/schema.yml
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
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
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
@@ -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
@@ -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,
- }
  }
  }
  }