Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
- Only emit `INSERT ... BY NAME` in the `replace_where`/`microbatch` strategies on DBR 18.0+ (and SQL warehouses), since older clusters reject the `BY NAME ... REPLACE WHERE` combination with a parse error ([1539](https://github.com/databricks/dbt-databricks/pull/1539) closes [#1532](https://github.com/databricks/dbt-databricks/issues/1532))
- Fix materialized views always rebuilding because Databricks-internal `tblproperties` were read as configuration drift; the diff now compares only the configured properties ([#1350](https://github.com/databricks/dbt-databricks/pull/1350) closes [#1314](https://github.com/databricks/dbt-databricks/issues/1314)).
- Stop metric views with `view_update_via_alter` from re-issuing a redundant `ALTER VIEW ... AS` on every run ([#1546](https://github.com/databricks/dbt-databricks/pull/1546))
- Fix column comments being permanently dropped from views when `view_update_via_alter` issues `ALTER VIEW AS`; reapply persisted column comments after the query update ([#1357](https://github.com/databricks/dbt-databricks/issues/1357))

### Under the Hood

Expand Down
6 changes: 6 additions & 0 deletions dbt/include/databricks/macros/relations/view/alter.sql
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@
{% endif %}
{% if query %}
{{ alter_query(target_relation, query.query) }}
{% if config.persist_column_docs() and model.columns %}
{#-- ALTER VIEW AS <query> wipes all column comments, so reapply them here. --#}
{%- set existing_columns = adapter.get_columns_in_relation(target_relation) -%}
{%- set columns_to_persist = adapter.get_persist_doc_columns(existing_columns, model.columns) -%}
{{ alter_column_comment(target_relation, columns_to_persist) }}
{% endif %}
{% endif %}
{% if column_comments %}
{{ alter_column_comments(target_relation, column_comments.comments) }}
Expand Down
31 changes: 31 additions & 0 deletions tests/functional/adapter/views/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,21 @@ def test_view_update_with_column_comments(self, project):
assert results[0][2] == "This is an id column"


class BaseUpdateQueryPreservesColumnComments(BaseUpdateView):
"""Regression for #1357: ALTER VIEW AS wipes column comments; they must be reapplied."""

def test_view_update_query_preserves_column_comments(self, project):
util.run_dbt(["build"])
util.write_file(fixtures.altered_view_sql, "models", "initial_view.sql")
util.run_dbt(["run"])

results = project.run_sql(
"describe extended {database}.{schema}.initial_view",
fetch="all",
)
assert results[0][2] == "This is the id column"


class BaseRemoveTags(BaseUpdateView):
def test_view_update_remove_tags(self, project):
util.run_dbt(["build"])
Expand Down Expand Up @@ -213,6 +228,22 @@ def project_config_update(self):
}


@pytest.mark.skip_profile("databricks_cluster")
class TestUpdateViewViaAlterQueryPreservesColumnComments(BaseUpdateQueryPreservesColumnComments):
@pytest.fixture(scope="class")
def project_config_update(self):
return {
"flags": {"use_materialization_v2": True},
"models": {
"+view_update_via_alter": True,
"+persist_docs": {
"relation": True,
"columns": True,
},
},
}


@pytest.mark.skip_profile("databricks_cluster")
class TestUpdateViewViaAlterRemoveTags(BaseRemoveTags):
@pytest.fixture(scope="class")
Expand Down
10 changes: 10 additions & 0 deletions tests/unit/macros/relations/test_view_macros.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def mocks(self, context):
context["apply_tags"] = Mock()
context["apply_tblproperties"] = Mock()
context["alter_query"] = Mock()
context["alter_column_comment"] = Mock()
context["apply_column_tags"] = Mock()

def render_alter_view(self, template_bundle, changes):
Expand Down Expand Up @@ -86,6 +87,15 @@ def test_macros__alter_view_with_query(self, context, template_bundle):
context["apply_tblproperties"].assert_not_called()
context["alter_query"].assert_called_once()

def test_macros__alter_view_with_query_reapplies_column_comments(
self, context, template_bundle
):
context["config"].persist_column_docs = Mock(return_value=True)
context["model"].columns = {"id": Mock()}
self.render_alter_view(template_bundle, {"query": Mock()})
context["alter_query"].assert_called_once()
context["alter_column_comment"].assert_called_once()

def test_macros__alter_view_with_column_tags(self, context, template_bundle):
column_tags = Mock()
self.render_alter_view(template_bundle, {"column_tags": column_tags})
Expand Down
Loading