From 2c9d170c26f715af9ea85f7c34eb8176543db0cf Mon Sep 17 00:00:00 2001 From: Rashi Jaiswal Date: Tue, 30 Jun 2026 14:33:45 +0530 Subject: [PATCH 1/9] Warn when documented columns are missing from the relation in persist_docs When persist_docs.columns is enabled, columns documented in a model's schema.yml but absent from the materialized relation were silently skipped by get_persist_doc_columns. Emit a warning naming those columns so users can catch typos and stale documentation. The columns are still filtered out (no behavior change to the comments that get applied). Ports the behavior added upstream in dbt-adapters#1684 (issue #1690). --- CHANGELOG.md | 1 + dbt/adapters/databricks/impl.py | 21 +++++++++++++++++++++ tests/unit/test_adapter.py | 25 +++++++++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45e634973..ba1befab0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### Fixes - Fix view materialization incorrectly producing a no-op instead of forcing recreation when `--full-refresh` is provided alongside `view_update_via_alter: true` and `use_materialization_v2: true` ([#1456](https://github.com/databricks/dbt-databricks/pull/1456) resolves [#1404](https://github.com/databricks/dbt-databricks/issues/1404)) +- Warn when a column documented in a model's `schema.yml` is absent from the relation while applying column comments, instead of silently skipping it — surfaces typos and stale column documentation. Covers the column-comment comparison paths (V1 `get_persist_doc_columns` and V2 `ColumnCommentsConfig.get_diff`); create-time inline comments are not yet covered. Ports the behavior added in dbt-adapters ([dbt-adapters#1684](https://github.com/dbt-labs/dbt-adapters/pull/1684) closes [dbt-adapters#1690](https://github.com/dbt-labs/dbt-adapters/issues/1690)) ([#1563](https://github.com/databricks/dbt-databricks/pull/1563)). - Support `dbt clone` and rebuilds over an existing shallow clone ([#1592](https://github.com/databricks/dbt-databricks/pull/1592) resolves [#1165](https://github.com/databricks/dbt-databricks/issues/1165)) - Fix managed Iceberg Python models failing with `MANAGED_TABLE_FORMAT` by emitting `.format("iceberg")` instead of the `parquet` sentinel from `resolve_file_format` (thanks @Divya-Kovvuru-0802!) ([#1593](https://github.com/databricks/dbt-databricks/pull/1593) resolves [#1591](https://github.com/databricks/dbt-databricks/issues/1591)) - Quote generated column identifiers in incremental strategies so non-ASCII column names no longer fail on subsequent runs ([#1595](https://github.com/databricks/dbt-databricks/pull/1595) resolves [#1594](https://github.com/databricks/dbt-databricks/issues/1594)) diff --git a/dbt/adapters/databricks/impl.py b/dbt/adapters/databricks/impl.py index 3e1069417..fc3745761 100644 --- a/dbt/adapters/databricks/impl.py +++ b/dbt/adapters/databricks/impl.py @@ -20,6 +20,7 @@ from dbt.adapters.catalogs import CatalogRelation from dbt.adapters.contracts.connection import AdapterResponse, Connection from dbt.adapters.contracts.relation import RelationConfig, RelationType +from dbt.adapters.events.types import AdapterEventWarning from dbt.adapters.relation_configs import RelationResults from dbt.adapters.spark.impl import ( DESCRIBE_TABLE_EXTENDED_MACRO_NAME, @@ -31,6 +32,7 @@ ) from dbt_common.behavior_flags import BehaviorFlag from dbt_common.contracts.config.base import BaseConfig, MergeBehavior +from dbt_common.events.functions import warn_or_error from dbt_common.exceptions import DbtConfigError, DbtInternalError, DbtRuntimeError from dbt_common.record import auto_record_function, record_function from dbt_common.utils import executor @@ -1008,6 +1010,25 @@ def get_persist_doc_columns( # Create a case-insensitive lookup for column names columns_lower = {k.lower(): k for k in columns.keys()} + # Warn about columns that are documented in the model's schema but are not present in the + # relation. These are silently skipped below (rather than erroring on the alter), so surface + # them to the user to catch typos and stale documentation. + existing_lower = {column.column.lower() for column in existing_columns} + missing = [ + original_name + for name_lower, original_name in columns_lower.items() + if name_lower not in existing_lower + ] + if missing: + warn_or_error( + AdapterEventWarning( + base_msg=( + "The following columns are specified in the schema but are not present " + "in the database and will be skipped: " + ", ".join(missing) + ) + ) + ) + for column in existing_columns: name = column.column # Use case-insensitive comparison for column names diff --git a/tests/unit/test_adapter.py b/tests/unit/test_adapter.py index 33c841983..2a4e948ae 100644 --- a/tests/unit/test_adapter.py +++ b/tests/unit/test_adapter.py @@ -1182,6 +1182,31 @@ def test_get_persist_doc_columns_case_mismatch_no_update_needed(self, adapter): # No update needed since comments match assert result == {} + @patch("dbt.adapters.databricks.impl.warn_or_error") + def test_get_persist_doc_columns_warns_on_missing_column(self, mock_warn, adapter): + """Documented columns absent from the relation are warned about and skipped.""" + existing = [self.create_column("col1", "comment1")] + column_dict = { + "col1": {"name": "col1", "description": "new comment"}, + "col2": {"name": "col2", "description": "comment for missing column"}, + } + result = adapter.get_persist_doc_columns(existing, column_dict) + # The missing column is filtered out; only the existing column is returned. + assert result == {"col1": {"name": "col1", "description": "new comment"}} + # A warning is emitted naming the missing column. + mock_warn.assert_called_once() + warned_event = mock_warn.call_args.args[0] + assert "col2" in warned_event.base_msg + assert "col1" not in warned_event.base_msg + + @patch("dbt.adapters.databricks.impl.warn_or_error") + def test_get_persist_doc_columns_no_warning_when_all_present(self, mock_warn, adapter): + """No warning is emitted when every documented column exists (case-insensitively).""" + existing = [self.create_column("Account_ID", "")] + column_dict = {"account_id": {"name": "account_id", "description": "Account ID column"}} + adapter.get_persist_doc_columns(existing, column_dict) + mock_warn.assert_not_called() + class TestGetColumnsByDbrVersion(DatabricksAdapterBase): @pytest.fixture From 66070280819732fd783e367ab6cb1c50d9c041d2 Mon Sep 17 00:00:00 2001 From: Rashi Jaiswal Date: Mon, 20 Jul 2026 16:22:47 +0530 Subject: [PATCH 2/9] Warn when documented columns are missing on the V2 materialization path The persist_docs missing-column warning only covered the V1 path (get_persist_doc_columns). The V2 (relation-config) path diffs column comments through ColumnCommentsConfig.get_diff, where a column documented in schema.yml but absent from the relation was still emitted into the diff (targeting a nonexistent column on the ALTER) with no feedback. Warn about those columns and skip them, matching the V1 behavior and the same warning message. Addresses review feedback on #1563. --- .../relation_configs/column_comments.py | 22 ++++++++++++ .../test_column_comments_config.py | 35 ++++++++++++++++++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/dbt/adapters/databricks/relation_configs/column_comments.py b/dbt/adapters/databricks/relation_configs/column_comments.py index 2f3487151..493304a59 100644 --- a/dbt/adapters/databricks/relation_configs/column_comments.py +++ b/dbt/adapters/databricks/relation_configs/column_comments.py @@ -1,7 +1,9 @@ from typing import ClassVar, Optional from dbt.adapters.contracts.relation import RelationConfig +from dbt.adapters.events.types import AdapterEventWarning from dbt.adapters.relation_configs.config_base import RelationResults +from dbt_common.events.functions import warn_or_error from dbt.adapters.databricks.logging import logger from dbt.adapters.databricks.relation_configs.base import ( @@ -23,8 +25,28 @@ def get_diff(self, other: "ColumnCommentsConfig") -> Optional["ColumnCommentsCon # Create a case-insensitive lookup for other's column comments other_comments_lower = {k.lower(): v for k, v in other.comments.items()} + # Warn about columns that are documented in the model's schema but are not present in + # the relation. These are skipped below (rather than erroring on the alter), so surface + # them to the user to catch typos and stale documentation. + missing = [ + column_name + for column_name in self.comments + if column_name.lower() not in other_comments_lower + ] + if missing: + warn_or_error( + AdapterEventWarning( + base_msg=( + "The following columns are specified in the schema but are not present " + "in the database and will be skipped: " + ", ".join(missing) + ) + ) + ) + for column_name, comment in self.comments.items(): # Use case-insensitive comparison for column names + if column_name.lower() not in other_comments_lower: + continue other_comment = other_comments_lower.get(column_name.lower()) if comment != other_comment: column_name = f"`{column_name}`" diff --git a/tests/unit/relation_configs/test_column_comments_config.py b/tests/unit/relation_configs/test_column_comments_config.py index d88440f3d..0f9948014 100644 --- a/tests/unit/relation_configs/test_column_comments_config.py +++ b/tests/unit/relation_configs/test_column_comments_config.py @@ -1,4 +1,4 @@ -from unittest.mock import Mock +from unittest.mock import Mock, patch from agate import Table @@ -102,3 +102,36 @@ def test_get_diff__case_mismatch_with_actual_changes(self): assert diff == ColumnCommentsConfig( comments={"`account_id`": "New Account ID"}, persist=True ) + + @patch("dbt.adapters.databricks.relation_configs.column_comments.warn_or_error") + def test_get_diff__warns_and_skips_missing_column(self, mock_warn): + """Documented columns absent from the relation are warned about and skipped.""" + # col2 is documented but not present in the relation + config = ColumnCommentsConfig( + comments={"col1": "new comment", "col2": "comment for missing column"}, persist=True + ) + other = ColumnCommentsConfig(comments={"col1": "old comment"}) + diff = config.get_diff(other) + # Only the existing column is included in the diff; the missing one is skipped. + assert diff == ColumnCommentsConfig(comments={"`col1`": "new comment"}, persist=True) + # A warning is emitted naming the missing column. + mock_warn.assert_called_once() + warned_event = mock_warn.call_args.args[0] + assert "col2" in warned_event.base_msg + assert "col1" not in warned_event.base_msg + + @patch("dbt.adapters.databricks.relation_configs.column_comments.warn_or_error") + def test_get_diff__no_warning_when_all_present(self, mock_warn): + """No warning is emitted when every documented column exists (case-insensitively).""" + config = ColumnCommentsConfig(comments={"account_id": "Account ID"}, persist=True) + other = ColumnCommentsConfig(comments={"Account_ID": ""}) + config.get_diff(other) + mock_warn.assert_not_called() + + @patch("dbt.adapters.databricks.relation_configs.column_comments.warn_or_error") + def test_get_diff__no_warning_when_not_persisting(self, mock_warn): + """Missing columns are not evaluated (or warned about) when persist is False.""" + config = ColumnCommentsConfig(comments={"col1": "comment", "col2": "comment"}) + other = ColumnCommentsConfig(comments={"col1": "comment"}) + assert config.get_diff(other) is None + mock_warn.assert_not_called() From a186cf0924032a1eaea19ebc2ea0b4f9f0d25a7c Mon Sep 17 00:00:00 2001 From: Rashi Jaiswal Date: Tue, 21 Jul 2026 16:14:38 +0530 Subject: [PATCH 3/9] test: functional coverage for persist_docs missing-column warning Add three functional tests exercising the missing-column warning end to end: - V1 comment path warns and still comments present columns - V2 alter path (get_diff) warns on a subsequent run - --warn-error escalates the warning to a run failure --- .../adapter/persist_docs/test_persist_docs.py | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/tests/functional/adapter/persist_docs/test_persist_docs.py b/tests/functional/adapter/persist_docs/test_persist_docs.py index 4c40be2f6..bf5d34ffe 100644 --- a/tests/functional/adapter/persist_docs/test_persist_docs.py +++ b/tests/functional/adapter/persist_docs/test_persist_docs.py @@ -419,3 +419,115 @@ def test_column_comment_suppressed_when_columns_false(self, adapter, table_relat f"v1 must suppress column comment when persist_docs.columns is false, " f"got {id_columns[0].comment!r}" ) + + +# Tail of the warning emitted when a documented column is absent from the relation. The offending +# column name (column_that_does_not_exist, from fixtures._PROPERTIES__SCHEMA_MISSING_COL) is +# asserted separately. +_MISSING_COLUMN_WARNING = "not present in the database and will be skipped" + + +class TestPersistDocsColumnMissingWarnsV1: + """v1: a documented column absent from the relation is warned about, not silently skipped. + + Complements TestPersistDocsColumnMissing (which only checks the run survives) by asserting the + warning names the offending column on the v1 comment path + (DatabricksAdapter.get_persist_doc_columns), and that present columns are still commented. + """ + + @pytest.fixture(scope="class") + def models(self): + return {"missing_column.sql": fixtures._MODELS__MISSING_COLUMN} + + @pytest.fixture(scope="class") + def properties(self): + return {"schema.yml": fixtures._PROPERTIES__SCHEMA_MISSING_COL} + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "flags": {"use_materialization_v2": False}, + "models": {"test": {"+persist_docs": {"relation": True, "columns": True}}}, + } + + @pytest.fixture(scope="class") + def table_relation(self, project): + return DatabricksRelation.create( + database=project.database, + schema=project.test_schema, + identifier="missing_column", + type="table", + ) + + def test_warns_and_still_comments_present_columns(self, adapter, table_relation): + _, logs = util.run_dbt_and_capture(["run"]) + assert _MISSING_COLUMN_WARNING in logs + assert "column_that_does_not_exist" in logs + + results = util.run_sql_with_adapter( + adapter, f"describe extended {table_relation}", fetch="all" + ) + _, columns = adapter.parse_describe_extended( + table_relation, Table(results, ["col_name", "data_type", "comment"]) + ) + id_columns = [c for c in columns if c.column == "id"] + assert id_columns and id_columns[0].comment + assert id_columns[0].comment.startswith("test id column description") + + +class TestPersistDocsColumnMissingWarnsV2: + """v2: the warning surfaces on the alter path (ColumnCommentsConfig.get_diff). + + On first create, comments are applied inline (parse_columns_and_constraints) and a + documented-but-absent column is silently dropped — get_diff is not consulted. The warning + therefore appears on a subsequent run, when documented columns are diffed against the existing + relation. (Create-time warning is tracked as a follow-up.) + """ + + @pytest.fixture(scope="class") + def models(self): + return {"missing_column.sql": fixtures._MODELS__MISSING_COLUMN} + + @pytest.fixture(scope="class") + def properties(self): + return {"schema.yml": fixtures._PROPERTIES__SCHEMA_MISSING_COL} + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "flags": {"use_materialization_v2": True}, + "models": {"test": {"+persist_docs": {"relation": True, "columns": True}}}, + } + + def test_warns_on_second_run(self, project): + # First run creates the relation; the inline-comment create path does not warn. + first_logs = util.run_dbt_and_capture(["run"])[1] + assert _MISSING_COLUMN_WARNING not in first_logs + + # Second run diffs documented columns against the existing relation → warns. + second_logs = util.run_dbt_and_capture(["run"])[1] + assert _MISSING_COLUMN_WARNING in second_logs + assert "column_that_does_not_exist" in second_logs + + +class TestPersistDocsColumnMissingWarnError: + """--warn-error escalates the missing-column warning to a run failure (v1 path).""" + + @pytest.fixture(scope="class") + def models(self): + return {"missing_column.sql": fixtures._MODELS__MISSING_COLUMN} + + @pytest.fixture(scope="class") + def properties(self): + return {"schema.yml": fixtures._PROPERTIES__SCHEMA_MISSING_COL} + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "flags": {"use_materialization_v2": False}, + "models": {"test": {"+persist_docs": {"relation": True, "columns": True}}}, + } + + def test_warn_error_fails_run(self, project): + _, logs = util.run_dbt_and_capture(["run", "--warn-error"], expect_pass=False) + assert "column_that_does_not_exist" in logs From 85ca1c3263df6629f7d1667adf0c907bdd2890a9 Mon Sep 17 00:00:00 2001 From: Rashi Jaiswal Date: Tue, 21 Jul 2026 16:22:58 +0530 Subject: [PATCH 4/9] test: use incremental model for v2 missing-column warning A table rebuild re-applies comments inline and never hits ColumnCommentsConfig.get_diff; the v2 changeset/alter path is only reached on a subsequent incremental run. Verified all three functional tests pass against a live UC SQL warehouse. --- .../adapter/persist_docs/fixtures.py | 19 +++++++++++++++++++ .../adapter/persist_docs/test_persist_docs.py | 13 +++++++------ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/tests/functional/adapter/persist_docs/fixtures.py b/tests/functional/adapter/persist_docs/fixtures.py index 1dd73e884..af2aa3141 100644 --- a/tests/functional/adapter/persist_docs/fixtures.py +++ b/tests/functional/adapter/persist_docs/fixtures.py @@ -39,6 +39,25 @@ select 1 as id, 'alice' as name """ +# Incremental model whose schema documents a column absent from the relation. Used to exercise the +# V2 alter/changeset path (ColumnCommentsConfig.get_diff), which — unlike a table rebuild — is only +# reached on a subsequent run against an existing relation. +missing_column_incremental_sql = """ +{{ config(materialized='incremental') }} +select 1 as id, 'Ed' as name +""" + +missing_column_incremental_schema = """ +version: 2 +models: + - name: missing_column_incremental + columns: + - name: id + description: "test id column description" + - name: column_that_does_not_exist + description: "comment that cannot be created" +""" + gate_model_schema = """ version: 2 models: diff --git a/tests/functional/adapter/persist_docs/test_persist_docs.py b/tests/functional/adapter/persist_docs/test_persist_docs.py index bf5d34ffe..db7515178 100644 --- a/tests/functional/adapter/persist_docs/test_persist_docs.py +++ b/tests/functional/adapter/persist_docs/test_persist_docs.py @@ -478,19 +478,20 @@ def test_warns_and_still_comments_present_columns(self, adapter, table_relation) class TestPersistDocsColumnMissingWarnsV2: """v2: the warning surfaces on the alter path (ColumnCommentsConfig.get_diff). - On first create, comments are applied inline (parse_columns_and_constraints) and a - documented-but-absent column is silently dropped — get_diff is not consulted. The warning - therefore appears on a subsequent run, when documented columns are diffed against the existing - relation. (Create-time warning is tracked as a follow-up.) + Uses an incremental model: a table rebuild re-applies comments inline and never consults + get_diff, so the changeset path is only reached on a subsequent incremental run, when + documented columns are diffed against the existing relation. On first create the + documented-but-absent column is silently dropped inline. (Create-time warning is tracked as a + follow-up.) """ @pytest.fixture(scope="class") def models(self): - return {"missing_column.sql": fixtures._MODELS__MISSING_COLUMN} + return {"missing_column_incremental.sql": override_fixtures.missing_column_incremental_sql} @pytest.fixture(scope="class") def properties(self): - return {"schema.yml": fixtures._PROPERTIES__SCHEMA_MISSING_COL} + return {"schema.yml": override_fixtures.missing_column_incremental_schema} @pytest.fixture(scope="class") def project_config_update(self): From 2841867e25c9dd82c47a5d6b04ec8082abdb5078 Mon Sep 17 00:00:00 2001 From: Rashi Jaiswal Date: Tue, 21 Jul 2026 16:28:48 +0530 Subject: [PATCH 5/9] test: assert missing-column warning is emitted exactly once The v1 and v2 warning sites are mutually exclusive per model run, so a single run warns exactly once. Lock that in as a regression guard against future double-warning if the warning is added to additional helpers. --- .../functional/adapter/persist_docs/test_persist_docs.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/functional/adapter/persist_docs/test_persist_docs.py b/tests/functional/adapter/persist_docs/test_persist_docs.py index db7515178..94c87fcb1 100644 --- a/tests/functional/adapter/persist_docs/test_persist_docs.py +++ b/tests/functional/adapter/persist_docs/test_persist_docs.py @@ -461,8 +461,9 @@ def table_relation(self, project): def test_warns_and_still_comments_present_columns(self, adapter, table_relation): _, logs = util.run_dbt_and_capture(["run"]) - assert _MISSING_COLUMN_WARNING in logs assert "column_that_does_not_exist" in logs + # Emitted exactly once — the v1 and v2 warning sites are mutually exclusive per run. + assert logs.count(_MISSING_COLUMN_WARNING) == 1 results = util.run_sql_with_adapter( adapter, f"describe extended {table_relation}", fetch="all" @@ -505,10 +506,11 @@ def test_warns_on_second_run(self, project): first_logs = util.run_dbt_and_capture(["run"])[1] assert _MISSING_COLUMN_WARNING not in first_logs - # Second run diffs documented columns against the existing relation → warns. + # Second run diffs documented columns against the existing relation → warns exactly once + # (get_diff runs once per component in a single get_changeset). second_logs = util.run_dbt_and_capture(["run"])[1] - assert _MISSING_COLUMN_WARNING in second_logs assert "column_that_does_not_exist" in second_logs + assert second_logs.count(_MISSING_COLUMN_WARNING) == 1 class TestPersistDocsColumnMissingWarnError: From d82949c364be987760034f683589c15778041b36 Mon Sep 17 00:00:00 2001 From: Shubham Dhal Date: Fri, 24 Jul 2026 14:08:12 +0530 Subject: [PATCH 6/9] fix(persist_docs): emit missing-column warning only once per model V1 incremental subsequent runs hit both get_diff and get_persist_doc_columns; dedupe via a shared thread-local helper, and cover columns-only + the double-warn path. --- CHANGELOG.md | 2 +- dbt/adapters/databricks/impl.py | 23 +++-- .../databricks/persist_doc_column_warnings.py | 54 ++++++++++++ .../relation_configs/column_comments.py | 15 +--- .../adapter/persist_docs/test_persist_docs.py | 87 ++++++++++++++++++- .../test_column_comments_config.py | 12 ++- tests/unit/test_adapter.py | 9 +- .../unit/test_persist_doc_column_warnings.py | 56 ++++++++++++ 8 files changed, 228 insertions(+), 30 deletions(-) create mode 100644 dbt/adapters/databricks/persist_doc_column_warnings.py create mode 100644 tests/unit/test_persist_doc_column_warnings.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ba1befab0..852418f76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ ### Fixes - Fix view materialization incorrectly producing a no-op instead of forcing recreation when `--full-refresh` is provided alongside `view_update_via_alter: true` and `use_materialization_v2: true` ([#1456](https://github.com/databricks/dbt-databricks/pull/1456) resolves [#1404](https://github.com/databricks/dbt-databricks/issues/1404)) -- Warn when a column documented in a model's `schema.yml` is absent from the relation while applying column comments, instead of silently skipping it — surfaces typos and stale column documentation. Covers the column-comment comparison paths (V1 `get_persist_doc_columns` and V2 `ColumnCommentsConfig.get_diff`); create-time inline comments are not yet covered. Ports the behavior added in dbt-adapters ([dbt-adapters#1684](https://github.com/dbt-labs/dbt-adapters/pull/1684) closes [dbt-adapters#1690](https://github.com/dbt-labs/dbt-adapters/issues/1690)) ([#1563](https://github.com/databricks/dbt-databricks/pull/1563)). +- Warn when a column documented in a model's `schema.yml` is absent from the relation while applying column comments, instead of silently skipping it — surfaces typos and stale column documentation. Covers the column-comment comparison paths (V1 `get_persist_doc_columns` and V2 `ColumnCommentsConfig.get_diff`); create-time inline comments are not yet covered. The warning is emitted at most once per unique missing-column set when both paths run in the same model (e.g. V1 incremental subsequent). Ports the behavior added in dbt-adapters ([dbt-adapters#1684](https://github.com/dbt-labs/dbt-adapters/pull/1684) closes [dbt-adapters#1690](https://github.com/dbt-labs/dbt-adapters/issues/1690)) ([#1563](https://github.com/databricks/dbt-databricks/pull/1563)). - Support `dbt clone` and rebuilds over an existing shallow clone ([#1592](https://github.com/databricks/dbt-databricks/pull/1592) resolves [#1165](https://github.com/databricks/dbt-databricks/issues/1165)) - Fix managed Iceberg Python models failing with `MANAGED_TABLE_FORMAT` by emitting `.format("iceberg")` instead of the `parquet` sentinel from `resolve_file_format` (thanks @Divya-Kovvuru-0802!) ([#1593](https://github.com/databricks/dbt-databricks/pull/1593) resolves [#1591](https://github.com/databricks/dbt-databricks/issues/1591)) - Quote generated column identifiers in incremental strategies so non-ASCII column names no longer fail on subsequent runs ([#1595](https://github.com/databricks/dbt-databricks/pull/1595) resolves [#1594](https://github.com/databricks/dbt-databricks/issues/1594)) diff --git a/dbt/adapters/databricks/impl.py b/dbt/adapters/databricks/impl.py index fc3745761..ac84fb14f 100644 --- a/dbt/adapters/databricks/impl.py +++ b/dbt/adapters/databricks/impl.py @@ -3,7 +3,7 @@ import re from abc import ABC, abstractmethod from collections import defaultdict -from collections.abc import Iterable, Iterator +from collections.abc import Iterable, Iterator, Mapping from concurrent.futures import Future from contextlib import contextmanager from dataclasses import dataclass, field @@ -20,7 +20,6 @@ from dbt.adapters.catalogs import CatalogRelation from dbt.adapters.contracts.connection import AdapterResponse, Connection from dbt.adapters.contracts.relation import RelationConfig, RelationType -from dbt.adapters.events.types import AdapterEventWarning from dbt.adapters.relation_configs import RelationResults from dbt.adapters.spark.impl import ( DESCRIBE_TABLE_EXTENDED_MACRO_NAME, @@ -32,7 +31,6 @@ ) from dbt_common.behavior_flags import BehaviorFlag from dbt_common.contracts.config.base import BaseConfig, MergeBehavior -from dbt_common.events.functions import warn_or_error from dbt_common.exceptions import DbtConfigError, DbtInternalError, DbtRuntimeError from dbt_common.record import auto_record_function, record_function from dbt_common.utils import executor @@ -59,6 +57,10 @@ from dbt.adapters.databricks.global_state import GlobalState from dbt.adapters.databricks.handle import SqlUtils from dbt.adapters.databricks.logging import logger +from dbt.adapters.databricks.persist_doc_column_warnings import ( + reset_missing_persist_doc_column_warnings, + warn_missing_persist_doc_columns, +) from dbt.adapters.databricks.python_models.python_submissions import ( AllPurposeClusterPythonJobHelper, JobClusterPythonJobHelper, @@ -904,6 +906,11 @@ def get_behavior_flag_no_warn(self, behavior_flag_name: str) -> bool: behavior_flag = getattr(self.behavior, behavior_flag_name) return behavior_flag.no_warn + def pre_model_hook(self, config: Mapping[str, Any]) -> Any: + """Reset missing-column warn dedupe so each model materialization can warn once.""" + reset_missing_persist_doc_column_warnings() + return super().pre_model_hook(config) + @available.parse(lambda *a, **k: (None, None)) @record_function( DatabricksAdapterAddQueryRecord, @@ -1019,15 +1026,7 @@ def get_persist_doc_columns( for name_lower, original_name in columns_lower.items() if name_lower not in existing_lower ] - if missing: - warn_or_error( - AdapterEventWarning( - base_msg=( - "The following columns are specified in the schema but are not present " - "in the database and will be skipped: " + ", ".join(missing) - ) - ) - ) + warn_missing_persist_doc_columns(missing) for column in existing_columns: name = column.column diff --git a/dbt/adapters/databricks/persist_doc_column_warnings.py b/dbt/adapters/databricks/persist_doc_column_warnings.py new file mode 100644 index 000000000..118aaca2e --- /dev/null +++ b/dbt/adapters/databricks/persist_doc_column_warnings.py @@ -0,0 +1,54 @@ +"""Shared warning for documented columns absent from the relation. + +Both the V1 persist_docs helper (``get_persist_doc_columns``) and the V2 changeset +helper (``ColumnCommentsConfig.get_diff``) need to surface the same user-facing +warning. On V1 incremental subsequent runs those two paths can both execute in a +single model materialization; dedupe so the message appears exactly once per +unique missing set **within that materialization**. + +State is thread-local: dbt assigns each model to one worker thread, so parallel +runs do not clear or suppress each other's warnings. ``pre_model_hook`` resets +the cache at the start of each model on that thread. +""" + +from __future__ import annotations + +import threading +from collections.abc import Sequence + +from dbt.adapters.events.types import AdapterEventWarning +from dbt_common.events.functions import warn_or_error + +_thread_state = threading.local() + + +def _emitted_keys() -> set[str]: + keys = getattr(_thread_state, "emitted_missing_keys", None) + if keys is None: + keys = set() + _thread_state.emitted_missing_keys = keys + return keys + + +def reset_missing_persist_doc_column_warnings() -> None: + """Clear the per-materialization dedupe cache for the current thread.""" + _thread_state.emitted_missing_keys = set() + + +def warn_missing_persist_doc_columns(missing: Sequence[str]) -> None: + """Warn once per unique set of documented-but-absent column names (per thread).""" + if not missing: + return + key = ", ".join(sorted(missing, key=str.lower)) + emitted = _emitted_keys() + if key in emitted: + return + emitted.add(key) + warn_or_error( + AdapterEventWarning( + base_msg=( + "The following columns are specified in the schema but are not present " + "in the database and will be skipped: " + ", ".join(missing) + ) + ) + ) diff --git a/dbt/adapters/databricks/relation_configs/column_comments.py b/dbt/adapters/databricks/relation_configs/column_comments.py index 493304a59..26f5d95f4 100644 --- a/dbt/adapters/databricks/relation_configs/column_comments.py +++ b/dbt/adapters/databricks/relation_configs/column_comments.py @@ -1,11 +1,12 @@ from typing import ClassVar, Optional from dbt.adapters.contracts.relation import RelationConfig -from dbt.adapters.events.types import AdapterEventWarning from dbt.adapters.relation_configs.config_base import RelationResults -from dbt_common.events.functions import warn_or_error from dbt.adapters.databricks.logging import logger +from dbt.adapters.databricks.persist_doc_column_warnings import ( + warn_missing_persist_doc_columns, +) from dbt.adapters.databricks.relation_configs.base import ( DatabricksComponentConfig, DatabricksComponentProcessor, @@ -33,15 +34,7 @@ def get_diff(self, other: "ColumnCommentsConfig") -> Optional["ColumnCommentsCon for column_name in self.comments if column_name.lower() not in other_comments_lower ] - if missing: - warn_or_error( - AdapterEventWarning( - base_msg=( - "The following columns are specified in the schema but are not present " - "in the database and will be skipped: " + ", ".join(missing) - ) - ) - ) + warn_missing_persist_doc_columns(missing) for column_name, comment in self.comments.items(): # Use case-insensitive comparison for column names diff --git a/tests/functional/adapter/persist_docs/test_persist_docs.py b/tests/functional/adapter/persist_docs/test_persist_docs.py index 94c87fcb1..555993d4a 100644 --- a/tests/functional/adapter/persist_docs/test_persist_docs.py +++ b/tests/functional/adapter/persist_docs/test_persist_docs.py @@ -462,7 +462,8 @@ def table_relation(self, project): def test_warns_and_still_comments_present_columns(self, adapter, table_relation): _, logs = util.run_dbt_and_capture(["run"]) assert "column_that_does_not_exist" in logs - # Emitted exactly once — the v1 and v2 warning sites are mutually exclusive per run. + # Emitted exactly once for this materialization + # (V1 table only hits get_persist_doc_columns). assert logs.count(_MISSING_COLUMN_WARNING) == 1 results = util.run_sql_with_adapter( @@ -534,3 +535,87 @@ def project_config_update(self): def test_warn_error_fails_run(self, project): _, logs = util.run_dbt_and_capture(["run", "--warn-error"], expect_pass=False) assert "column_that_does_not_exist" in logs + + +class TestPersistDocsColumnMissingWarnsV1ColumnsOnly: + """v1: columns-only persist_docs still warns (does not require relation: true).""" + + @pytest.fixture(scope="class") + def models(self): + return {"missing_column.sql": fixtures._MODELS__MISSING_COLUMN} + + @pytest.fixture(scope="class") + def properties(self): + return {"schema.yml": fixtures._PROPERTIES__SCHEMA_MISSING_COL} + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "flags": {"use_materialization_v2": False}, + "models": {"test": {"+persist_docs": {"relation": False, "columns": True}}}, + } + + @pytest.fixture(scope="class") + def table_relation(self, project): + return DatabricksRelation.create( + database=project.database, + schema=project.test_schema, + identifier="missing_column", + type="table", + ) + + def test_warns_once_with_columns_only(self, adapter, table_relation): + _, logs = util.run_dbt_and_capture(["run"]) + assert "column_that_does_not_exist" in logs + assert logs.count(_MISSING_COLUMN_WARNING) == 1 + + results = util.run_sql_with_adapter( + adapter, f"describe extended {table_relation}", fetch="all" + ) + _, columns = adapter.parse_describe_extended( + table_relation, Table(results, ["col_name", "data_type", "comment"]) + ) + id_columns = [c for c in columns if c.column == "id"] + assert id_columns and id_columns[0].comment + assert id_columns[0].comment.startswith("test id column description") + + +class TestPersistDocsColumnMissingWarnsOnceV1IncrementalSubsequent: + """v1 incremental subsequent with both flags must warn exactly once. + + On V1 subsequent runs, get_changeset may call ColumnCommentsConfig.get_diff (warn) and + persist_docs later calls get_persist_doc_columns (warn). Those must not double-fire. + """ + + @pytest.fixture(scope="class") + def models(self): + return {"missing_column_incremental.sql": override_fixtures.missing_column_incremental_sql} + + @pytest.fixture(scope="class") + def properties(self): + return {"schema.yml": override_fixtures.missing_column_incremental_schema} + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "flags": {"use_materialization_v2": False}, + "models": { + "test": { + "+persist_docs": {"relation": True, "columns": True}, + "+incremental_apply_config_changes": True, + } + }, + } + + def test_subsequent_run_warns_exactly_once(self, project): + # First run creates via persist_docs → get_persist_doc_columns (warns once). + first_logs = util.run_dbt_and_capture(["run"])[1] + assert first_logs.count(_MISSING_COLUMN_WARNING) == 1 + + # Second run hits get_diff (changeset) AND persist_docs; must still be exactly once. + second_logs = util.run_dbt_and_capture(["run"])[1] + assert "column_that_does_not_exist" in second_logs + assert second_logs.count(_MISSING_COLUMN_WARNING) == 1, ( + f"Expected exactly 1 missing-column warning on V1 incremental subsequent, " + f"found {second_logs.count(_MISSING_COLUMN_WARNING)}. Logs:\n{second_logs}" + ) diff --git a/tests/unit/relation_configs/test_column_comments_config.py b/tests/unit/relation_configs/test_column_comments_config.py index 0f9948014..c7b528e30 100644 --- a/tests/unit/relation_configs/test_column_comments_config.py +++ b/tests/unit/relation_configs/test_column_comments_config.py @@ -2,6 +2,9 @@ from agate import Table +from dbt.adapters.databricks.persist_doc_column_warnings import ( + reset_missing_persist_doc_column_warnings, +) from dbt.adapters.databricks.relation_configs.column_comments import ( ColumnCommentsConfig, ColumnCommentsProcessor, @@ -103,9 +106,10 @@ def test_get_diff__case_mismatch_with_actual_changes(self): comments={"`account_id`": "New Account ID"}, persist=True ) - @patch("dbt.adapters.databricks.relation_configs.column_comments.warn_or_error") + @patch("dbt.adapters.databricks.persist_doc_column_warnings.warn_or_error") def test_get_diff__warns_and_skips_missing_column(self, mock_warn): """Documented columns absent from the relation are warned about and skipped.""" + reset_missing_persist_doc_column_warnings() # col2 is documented but not present in the relation config = ColumnCommentsConfig( comments={"col1": "new comment", "col2": "comment for missing column"}, persist=True @@ -120,17 +124,19 @@ def test_get_diff__warns_and_skips_missing_column(self, mock_warn): assert "col2" in warned_event.base_msg assert "col1" not in warned_event.base_msg - @patch("dbt.adapters.databricks.relation_configs.column_comments.warn_or_error") + @patch("dbt.adapters.databricks.persist_doc_column_warnings.warn_or_error") def test_get_diff__no_warning_when_all_present(self, mock_warn): """No warning is emitted when every documented column exists (case-insensitively).""" + reset_missing_persist_doc_column_warnings() config = ColumnCommentsConfig(comments={"account_id": "Account ID"}, persist=True) other = ColumnCommentsConfig(comments={"Account_ID": ""}) config.get_diff(other) mock_warn.assert_not_called() - @patch("dbt.adapters.databricks.relation_configs.column_comments.warn_or_error") + @patch("dbt.adapters.databricks.persist_doc_column_warnings.warn_or_error") def test_get_diff__no_warning_when_not_persisting(self, mock_warn): """Missing columns are not evaluated (or warned about) when persist is False.""" + reset_missing_persist_doc_column_warnings() config = ColumnCommentsConfig(comments={"col1": "comment", "col2": "comment"}) other = ColumnCommentsConfig(comments={"col1": "comment"}) assert config.get_diff(other) is None diff --git a/tests/unit/test_adapter.py b/tests/unit/test_adapter.py index 2a4e948ae..44b5af4fd 100644 --- a/tests/unit/test_adapter.py +++ b/tests/unit/test_adapter.py @@ -34,6 +34,9 @@ ViewAPI, get_identifier_list_string, ) +from dbt.adapters.databricks.persist_doc_column_warnings import ( + reset_missing_persist_doc_column_warnings, +) from dbt.adapters.databricks.relation import ( DatabricksRelation, DatabricksRelationType, @@ -1182,9 +1185,10 @@ def test_get_persist_doc_columns_case_mismatch_no_update_needed(self, adapter): # No update needed since comments match assert result == {} - @patch("dbt.adapters.databricks.impl.warn_or_error") + @patch("dbt.adapters.databricks.persist_doc_column_warnings.warn_or_error") def test_get_persist_doc_columns_warns_on_missing_column(self, mock_warn, adapter): """Documented columns absent from the relation are warned about and skipped.""" + reset_missing_persist_doc_column_warnings() existing = [self.create_column("col1", "comment1")] column_dict = { "col1": {"name": "col1", "description": "new comment"}, @@ -1199,9 +1203,10 @@ def test_get_persist_doc_columns_warns_on_missing_column(self, mock_warn, adapte assert "col2" in warned_event.base_msg assert "col1" not in warned_event.base_msg - @patch("dbt.adapters.databricks.impl.warn_or_error") + @patch("dbt.adapters.databricks.persist_doc_column_warnings.warn_or_error") def test_get_persist_doc_columns_no_warning_when_all_present(self, mock_warn, adapter): """No warning is emitted when every documented column exists (case-insensitively).""" + reset_missing_persist_doc_column_warnings() existing = [self.create_column("Account_ID", "")] column_dict = {"account_id": {"name": "account_id", "description": "Account ID column"}} adapter.get_persist_doc_columns(existing, column_dict) diff --git a/tests/unit/test_persist_doc_column_warnings.py b/tests/unit/test_persist_doc_column_warnings.py new file mode 100644 index 000000000..523a3cdfa --- /dev/null +++ b/tests/unit/test_persist_doc_column_warnings.py @@ -0,0 +1,56 @@ +"""Unit tests for missing documented-column warning dedupe.""" + +import threading +from unittest.mock import patch + +from dbt.adapters.databricks.persist_doc_column_warnings import ( + reset_missing_persist_doc_column_warnings, + warn_missing_persist_doc_columns, +) +from dbt.adapters.databricks.relation_configs.column_comments import ColumnCommentsConfig + + +class TestWarnMissingPersistDocColumnsDedupe: + def setup_method(self) -> None: + reset_missing_persist_doc_column_warnings() + + @patch("dbt.adapters.databricks.persist_doc_column_warnings.warn_or_error") + def test_warns_once_for_same_missing_set(self, mock_warn): + warn_missing_persist_doc_columns(["col2"]) + warn_missing_persist_doc_columns(["col2"]) + mock_warn.assert_called_once() + + @patch("dbt.adapters.databricks.persist_doc_column_warnings.warn_or_error") + def test_get_diff_and_helper_share_dedupe(self, mock_warn): + """get_diff then the shared helper must not double-fire for the same cols.""" + config = ColumnCommentsConfig(comments={"col1": "new", "col2": "missing"}, persist=True) + other = ColumnCommentsConfig(comments={"col1": "old"}) + config.get_diff(other) + warn_missing_persist_doc_columns(["col2"]) + mock_warn.assert_called_once() + assert "col2" in mock_warn.call_args.args[0].base_msg + + @patch("dbt.adapters.databricks.persist_doc_column_warnings.warn_or_error") + def test_different_missing_sets_warn_separately(self, mock_warn): + warn_missing_persist_doc_columns(["col2"]) + warn_missing_persist_doc_columns(["col3"]) + assert mock_warn.call_count == 2 + + @patch("dbt.adapters.databricks.persist_doc_column_warnings.warn_or_error") + def test_thread_local_isolation(self, mock_warn): + """A reset on another thread must not suppress warnings on this thread.""" + warn_missing_persist_doc_columns(["col2"]) + assert mock_warn.call_count == 1 + + def other_thread() -> None: + reset_missing_persist_doc_column_warnings() + warn_missing_persist_doc_columns(["col2"]) + + t = threading.Thread(target=other_thread) + t.start() + t.join() + # Main thread already warned once; other thread warns independently → 2 total. + assert mock_warn.call_count == 2 + # Main thread still dedupes its own second call. + warn_missing_persist_doc_columns(["col2"]) + assert mock_warn.call_count == 2 From 41432c9ae884cc481cec0637a4a39d5bf6a4eec4 Mon Sep 17 00:00:00 2001 From: Shubham Dhal Date: Fri, 24 Jul 2026 14:31:22 +0530 Subject: [PATCH 7/9] chore: drop CHANGELOG tweak from single-warn follow-up --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 852418f76..ba1befab0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ ### Fixes - Fix view materialization incorrectly producing a no-op instead of forcing recreation when `--full-refresh` is provided alongside `view_update_via_alter: true` and `use_materialization_v2: true` ([#1456](https://github.com/databricks/dbt-databricks/pull/1456) resolves [#1404](https://github.com/databricks/dbt-databricks/issues/1404)) -- Warn when a column documented in a model's `schema.yml` is absent from the relation while applying column comments, instead of silently skipping it — surfaces typos and stale column documentation. Covers the column-comment comparison paths (V1 `get_persist_doc_columns` and V2 `ColumnCommentsConfig.get_diff`); create-time inline comments are not yet covered. The warning is emitted at most once per unique missing-column set when both paths run in the same model (e.g. V1 incremental subsequent). Ports the behavior added in dbt-adapters ([dbt-adapters#1684](https://github.com/dbt-labs/dbt-adapters/pull/1684) closes [dbt-adapters#1690](https://github.com/dbt-labs/dbt-adapters/issues/1690)) ([#1563](https://github.com/databricks/dbt-databricks/pull/1563)). +- Warn when a column documented in a model's `schema.yml` is absent from the relation while applying column comments, instead of silently skipping it — surfaces typos and stale column documentation. Covers the column-comment comparison paths (V1 `get_persist_doc_columns` and V2 `ColumnCommentsConfig.get_diff`); create-time inline comments are not yet covered. Ports the behavior added in dbt-adapters ([dbt-adapters#1684](https://github.com/dbt-labs/dbt-adapters/pull/1684) closes [dbt-adapters#1690](https://github.com/dbt-labs/dbt-adapters/issues/1690)) ([#1563](https://github.com/databricks/dbt-databricks/pull/1563)). - Support `dbt clone` and rebuilds over an existing shallow clone ([#1592](https://github.com/databricks/dbt-databricks/pull/1592) resolves [#1165](https://github.com/databricks/dbt-databricks/issues/1165)) - Fix managed Iceberg Python models failing with `MANAGED_TABLE_FORMAT` by emitting `.format("iceberg")` instead of the `parquet` sentinel from `resolve_file_format` (thanks @Divya-Kovvuru-0802!) ([#1593](https://github.com/databricks/dbt-databricks/pull/1593) resolves [#1591](https://github.com/databricks/dbt-databricks/issues/1591)) - Quote generated column identifiers in incremental strategies so non-ASCII column names no longer fail on subsequent runs ([#1595](https://github.com/databricks/dbt-databricks/pull/1595) resolves [#1594](https://github.com/databricks/dbt-databricks/issues/1594)) From a6ee456ae6cbc22b07fe7deb32cd9f211bd0920c Mon Sep 17 00:00:00 2001 From: Rashi Jaiswal Date: Tue, 28 Jul 2026 11:44:18 +0530 Subject: [PATCH 8/9] Validate persist_docs columns post-build; gate v2 column comments on persist_docs.columns --- CHANGELOG.md | 2 +- dbt/adapters/databricks/impl.py | 35 +++++-- .../relation_configs/column_comments.py | 24 ++--- .../macros/adapters/persist_docs.sql | 17 ++++ .../incremental/incremental.sql | 4 + .../macros/materializations/table.sql | 3 + .../adapter/persist_docs/test_persist_docs.py | 94 ++++++++++++++++--- .../test_column_comments_config.py | 42 +++++---- tests/unit/test_adapter.py | 39 ++++++++ .../unit/test_persist_doc_column_warnings.py | 8 +- 10 files changed, 213 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba1befab0..ab47427e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ ### Fixes - Fix view materialization incorrectly producing a no-op instead of forcing recreation when `--full-refresh` is provided alongside `view_update_via_alter: true` and `use_materialization_v2: true` ([#1456](https://github.com/databricks/dbt-databricks/pull/1456) resolves [#1404](https://github.com/databricks/dbt-databricks/issues/1404)) -- Warn when a column documented in a model's `schema.yml` is absent from the relation while applying column comments, instead of silently skipping it — surfaces typos and stale column documentation. Covers the column-comment comparison paths (V1 `get_persist_doc_columns` and V2 `ColumnCommentsConfig.get_diff`); create-time inline comments are not yet covered. Ports the behavior added in dbt-adapters ([dbt-adapters#1684](https://github.com/dbt-labs/dbt-adapters/pull/1684) closes [dbt-adapters#1690](https://github.com/dbt-labs/dbt-adapters/issues/1690)) ([#1563](https://github.com/databricks/dbt-databricks/pull/1563)). +- Warn when a column documented in a model's `schema.yml` is absent from the relation, instead of silently skipping it — surfaces typos and stale column documentation. The check runs post-build against the actual relation (matching the shared `validate_doc_columns` behavior the other adapters use), so it covers V1 and V2 table/incremental on both the initial create and subsequent runs, does not false-warn on a legitimately new column, and is gated on `persist_docs.columns`. Also fixes the V2 column-comment gate to key off `persist_docs.columns` rather than `persist_docs.relation`. Materialized-view/streaming-table and view create are tracked as follow-ups. Ports the behavior added in dbt-adapters ([dbt-adapters#1684](https://github.com/dbt-labs/dbt-adapters/pull/1684) closes [dbt-adapters#1690](https://github.com/dbt-labs/dbt-adapters/issues/1690)) ([#1563](https://github.com/databricks/dbt-databricks/pull/1563)). - Support `dbt clone` and rebuilds over an existing shallow clone ([#1592](https://github.com/databricks/dbt-databricks/pull/1592) resolves [#1165](https://github.com/databricks/dbt-databricks/issues/1165)) - Fix managed Iceberg Python models failing with `MANAGED_TABLE_FORMAT` by emitting `.format("iceberg")` instead of the `parquet` sentinel from `resolve_file_format` (thanks @Divya-Kovvuru-0802!) ([#1593](https://github.com/databricks/dbt-databricks/pull/1593) resolves [#1591](https://github.com/databricks/dbt-databricks/issues/1591)) - Quote generated column identifiers in incremental strategies so non-ASCII column names no longer fail on subsequent runs ([#1595](https://github.com/databricks/dbt-databricks/pull/1595) resolves [#1594](https://github.com/databricks/dbt-databricks/issues/1594)) diff --git a/dbt/adapters/databricks/impl.py b/dbt/adapters/databricks/impl.py index ac84fb14f..d8823a6db 100644 --- a/dbt/adapters/databricks/impl.py +++ b/dbt/adapters/databricks/impl.py @@ -1003,6 +1003,26 @@ def _catalog(self, catalog: Optional[str]) -> Iterator[None]: if current_catalog is not None: self.execute_macro(USE_CATALOG_MACRO_NAME, kwargs=dict(catalog=current_catalog)) + @staticmethod + def _find_missing_doc_columns( + existing_columns: list[DatabricksColumn], columns: dict[str, Any] + ) -> list[str]: + """Documented column names (from the model) that are absent from the relation.""" + existing_lower = {column.column.lower() for column in existing_columns} + return [name for name in columns if name.lower() not in existing_lower] + + @available.parse(lambda *a, **k: None) + def validate_persist_doc_columns( + self, existing_columns: list[DatabricksColumn], columns: dict[str, Any] + ) -> None: + """Warn about documented columns that are absent from the relation. + + Mirrors the shared ``validate_doc_columns`` behavior the other adapters use: the check runs + post-build against the actual relation, so a legitimately new column (present in the model + and the freshly-built relation) is not flagged. Applies no comments. + """ + warn_missing_persist_doc_columns(self._find_missing_doc_columns(existing_columns, columns)) + @available.parse(lambda *a, **k: {}) def get_persist_doc_columns( self, existing_columns: list[DatabricksColumn], columns: dict[str, Any] @@ -1017,16 +1037,11 @@ def get_persist_doc_columns( # Create a case-insensitive lookup for column names columns_lower = {k.lower(): k for k in columns.keys()} - # Warn about columns that are documented in the model's schema but are not present in the - # relation. These are silently skipped below (rather than erroring on the alter), so surface - # them to the user to catch typos and stale documentation. - existing_lower = {column.column.lower() for column in existing_columns} - missing = [ - original_name - for name_lower, original_name in columns_lower.items() - if name_lower not in existing_lower - ] - warn_missing_persist_doc_columns(missing) + # Documented-but-absent columns are skipped below (rather than erroring on the alter); warn + # so the user can catch typos and stale documentation. This runs post-build (V1 persist_docs + # gathers existing_columns from the written relation), so a legitimately new column is not + # flagged. + warn_missing_persist_doc_columns(self._find_missing_doc_columns(existing_columns, columns)) for column in existing_columns: name = column.column diff --git a/dbt/adapters/databricks/relation_configs/column_comments.py b/dbt/adapters/databricks/relation_configs/column_comments.py index 26f5d95f4..8c848a911 100644 --- a/dbt/adapters/databricks/relation_configs/column_comments.py +++ b/dbt/adapters/databricks/relation_configs/column_comments.py @@ -4,9 +4,6 @@ from dbt.adapters.relation_configs.config_base import RelationResults from dbt.adapters.databricks.logging import logger -from dbt.adapters.databricks.persist_doc_column_warnings import ( - warn_missing_persist_doc_columns, -) from dbt.adapters.databricks.relation_configs.base import ( DatabricksComponentConfig, DatabricksComponentProcessor, @@ -26,18 +23,12 @@ def get_diff(self, other: "ColumnCommentsConfig") -> Optional["ColumnCommentsCon # Create a case-insensitive lookup for other's column comments other_comments_lower = {k.lower(): v for k, v in other.comments.items()} - # Warn about columns that are documented in the model's schema but are not present in - # the relation. These are skipped below (rather than erroring on the alter), so surface - # them to the user to catch typos and stale documentation. - missing = [ - column_name - for column_name in self.comments - if column_name.lower() not in other_comments_lower - ] - warn_missing_persist_doc_columns(missing) - for column_name, comment in self.comments.items(): - # Use case-insensitive comparison for column names + # Use case-insensitive comparison for column names. Documented columns that are + # absent from the relation are skipped here so the alter never targets a nonexistent + # column; the user-facing "missing column" warning is emitted post-build by + # validate_persist_doc_columns (against the actual relation), so a legitimately new + # column is not flagged before it has been materialized. if column_name.lower() not in other_comments_lower: continue other_comment = other_comments_lower.get(column_name.lower()) @@ -68,7 +59,10 @@ def from_relation_config(cls, relation_config: RelationConfig) -> ColumnComments columns = getattr(relation_config, "columns", {}) persist = False if relation_config.config: - persist = relation_config.config.persist_docs.get("relation") or False + # Column comments are gated on persist_docs.columns (the column-level knob), matching + # config.persist_column_docs() used by the V1 persist_docs / view-create / seed paths. + # persist_docs.relation is the table-comment knob and is the wrong gate here. + persist = relation_config.config.persist_docs.get("columns") or False comments = {} for column_name, column in columns.items(): if hasattr(column, "description"): diff --git a/dbt/include/databricks/macros/adapters/persist_docs.sql b/dbt/include/databricks/macros/adapters/persist_docs.sql index da3734ea1..029c2d5f9 100644 --- a/dbt/include/databricks/macros/adapters/persist_docs.sql +++ b/dbt/include/databricks/macros/adapters/persist_docs.sql @@ -42,6 +42,23 @@ {% endif %} {% endmacro %} +{#-- + Post-build validation of documented column comments against the actual relation. + + The V2 materialization path applies column comments inline at create-time and via the + relation-config diff (neither of which sees the model's documented columns as a set), so this + runs after the relation is built to surface columns that are documented in the schema but absent + from the relation (typos / stale docs). It mirrors the shared validate_doc_columns behavior the + other adapters use, and applies no comments itself. Gated on persist_docs.columns so it never + fires when column persistence is disabled (avoids --warn-error false failures). +--#} +{% macro validate_persist_doc_columns(relation, model) -%} + {% if config.persist_column_docs() and model.columns %} + {%- set existing_columns = adapter.get_columns_in_relation(relation) -%} + {%- do adapter.validate_persist_doc_columns(existing_columns, model.columns) -%} + {% endif %} +{%- endmacro %} + {% macro alter_relation_comment_sql(relation, description) %} COMMENT ON {{ relation.type.render().upper() }} {{ relation.render() }} IS '{{ description | replace("'", "\\'") }}' {% endmacro %} diff --git a/dbt/include/databricks/macros/materializations/incremental/incremental.sql b/dbt/include/databricks/macros/materializations/incremental/incremental.sql index e79859822..5339a723c 100644 --- a/dbt/include/databricks/macros/materializations/incremental/incremental.sql +++ b/dbt/include/databricks/macros/materializations/incremental/incremental.sql @@ -81,6 +81,10 @@ {%- endif -%} {%- endif -%} + {#-- Warn (post-build) about documented columns absent from the final relation. Runs on every + sub-branch above and regardless of incremental_apply_config_changes. --#} + {% do validate_persist_doc_columns(target_relation, model) %} + {% set should_revoke = should_revoke(existing_relation, full_refresh_mode) %} {% do apply_grants(target_relation, grant_config, should_revoke) %} {% do optimize(target_relation) %} diff --git a/dbt/include/databricks/macros/materializations/table.sql b/dbt/include/databricks/macros/materializations/table.sql index eee612c36..32a411681 100644 --- a/dbt/include/databricks/macros/materializations/table.sql +++ b/dbt/include/databricks/macros/materializations/table.sql @@ -32,6 +32,9 @@ {% endif %} {% endif %} + {#-- Warn (post-build) about documented columns absent from the final relation. --#} + {% do validate_persist_doc_columns(target_relation, model) %} + {% set should_revoke = should_revoke(existing_relation, full_refresh_mode=True) %} {{ apply_grants(target_relation, grant_config, should_revoke) }} {% do optimize(target_relation) %} diff --git a/tests/functional/adapter/persist_docs/test_persist_docs.py b/tests/functional/adapter/persist_docs/test_persist_docs.py index 555993d4a..601f4d1df 100644 --- a/tests/functional/adapter/persist_docs/test_persist_docs.py +++ b/tests/functional/adapter/persist_docs/test_persist_docs.py @@ -478,13 +478,12 @@ def test_warns_and_still_comments_present_columns(self, adapter, table_relation) class TestPersistDocsColumnMissingWarnsV2: - """v2: the warning surfaces on the alter path (ColumnCommentsConfig.get_diff). + """v2: a documented column absent from the relation is warned about post-build. - Uses an incremental model: a table rebuild re-applies comments inline and never consults - get_diff, so the changeset path is only reached on a subsequent incremental run, when - documented columns are diffed against the existing relation. On first create the - documented-but-absent column is silently dropped inline. (Create-time warning is tracked as a - follow-up.) + The warning is emitted by validate_persist_doc_columns after the relation is built (mirroring + the shared validate_doc_columns behavior the other adapters use), not from the + pre-materialization changeset diff. So it fires on the initial create and on every subsequent + incremental run — and a legitimately new column (present post-build) would not warn. """ @pytest.fixture(scope="class") @@ -502,18 +501,67 @@ def project_config_update(self): "models": {"test": {"+persist_docs": {"relation": True, "columns": True}}}, } - def test_warns_on_second_run(self, project): - # First run creates the relation; the inline-comment create path does not warn. + def test_warns_on_create_and_subsequent_runs(self, project): + # Post-build validation warns on the initial create... first_logs = util.run_dbt_and_capture(["run"])[1] - assert _MISSING_COLUMN_WARNING not in first_logs + assert "column_that_does_not_exist" in first_logs + assert first_logs.count(_MISSING_COLUMN_WARNING) == 1 - # Second run diffs documented columns against the existing relation → warns exactly once - # (get_diff runs once per component in a single get_changeset). + # ...and again on a subsequent incremental run, still exactly once (no double-warn). second_logs = util.run_dbt_and_capture(["run"])[1] assert "column_that_does_not_exist" in second_logs assert second_logs.count(_MISSING_COLUMN_WARNING) == 1 +class TestPersistDocsColumnMissingWarnsV2ColumnsOnly: + """v2 columns-only persist_docs warns (E2: column comments gate on persist_docs.columns, + not .relation — previously this combination was silent on the v2 path).""" + + @pytest.fixture(scope="class") + def models(self): + return {"missing_column_incremental.sql": override_fixtures.missing_column_incremental_sql} + + @pytest.fixture(scope="class") + def properties(self): + return {"schema.yml": override_fixtures.missing_column_incremental_schema} + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "flags": {"use_materialization_v2": True}, + "models": {"test": {"+persist_docs": {"relation": False, "columns": True}}}, + } + + def test_warns_with_columns_only(self, project): + _, logs = util.run_dbt_and_capture(["run"]) + assert "column_that_does_not_exist" in logs + assert logs.count(_MISSING_COLUMN_WARNING) == 1 + + +class TestPersistDocsColumnMissingV2RelationOnlyNoWarn: + """v2 with columns:false does no column-doc work, so the missing-column check stays silent + (E2: column comments are gated on persist_docs.columns).""" + + @pytest.fixture(scope="class") + def models(self): + return {"missing_column_incremental.sql": override_fixtures.missing_column_incremental_sql} + + @pytest.fixture(scope="class") + def properties(self): + return {"schema.yml": override_fixtures.missing_column_incremental_schema} + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "flags": {"use_materialization_v2": True}, + "models": {"test": {"+persist_docs": {"relation": True, "columns": False}}}, + } + + def test_no_warning_when_columns_disabled(self, project): + _, logs = util.run_dbt_and_capture(["run"]) + assert _MISSING_COLUMN_WARNING not in logs + + class TestPersistDocsColumnMissingWarnError: """--warn-error escalates the missing-column warning to a run failure (v1 path).""" @@ -537,6 +585,30 @@ def test_warn_error_fails_run(self, project): assert "column_that_does_not_exist" in logs +class TestPersistDocsColumnMissingWarnErrorV2: + """--warn-error escalates the post-build missing-column warning to a run failure (v2 path).""" + + @pytest.fixture(scope="class") + def models(self): + return {"missing_column_incremental.sql": override_fixtures.missing_column_incremental_sql} + + @pytest.fixture(scope="class") + def properties(self): + return {"schema.yml": override_fixtures.missing_column_incremental_schema} + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "flags": {"use_materialization_v2": True}, + "models": {"test": {"+persist_docs": {"relation": True, "columns": True}}}, + } + + def test_warn_error_fails_run(self, project): + # V2 create goes through the post-build validation; --warn-error must fail the run. + _, logs = util.run_dbt_and_capture(["run", "--warn-error"], expect_pass=False) + assert "column_that_does_not_exist" in logs + + class TestPersistDocsColumnMissingWarnsV1ColumnsOnly: """v1: columns-only persist_docs still warns (does not require relation: true).""" diff --git a/tests/unit/relation_configs/test_column_comments_config.py b/tests/unit/relation_configs/test_column_comments_config.py index c7b528e30..c56aaf514 100644 --- a/tests/unit/relation_configs/test_column_comments_config.py +++ b/tests/unit/relation_configs/test_column_comments_config.py @@ -48,10 +48,27 @@ def test_from_relation_config__no_persist(self): def test_from_relation_config__with_persist(self): model = Mock() model.columns = {"col1": {"description": "test comment"}} - model.config.persist_docs = {"relation": True} + # Column comments are gated on persist_docs.columns, not .relation. + model.config.persist_docs = {"columns": True} config = ColumnCommentsProcessor.from_relation_config(model) assert config == ColumnCommentsConfig(comments={"col1": "test comment"}, persist=True) + def test_from_relation_config__columns_true_relation_false(self): + """persist_docs.columns drives column comments even when .relation is false.""" + model = Mock() + model.columns = {"col1": {"description": "test comment"}} + model.config.persist_docs = {"columns": True, "relation": False} + config = ColumnCommentsProcessor.from_relation_config(model) + assert config == ColumnCommentsConfig(comments={"col1": "test comment"}, persist=True) + + def test_from_relation_config__relation_true_columns_false(self): + """Column comments are not applied when .columns is off, even if .relation is on.""" + model = Mock() + model.columns = {"col1": {"description": "test comment"}} + model.config.persist_docs = {"relation": True, "columns": False} + config = ColumnCommentsProcessor.from_relation_config(model) + assert config == ColumnCommentsConfig(comments={"col1": "test comment"}, persist=False) + class TestColumnCommentsConfig: def test_get_diff__no_changes(self): @@ -107,8 +124,13 @@ def test_get_diff__case_mismatch_with_actual_changes(self): ) @patch("dbt.adapters.databricks.persist_doc_column_warnings.warn_or_error") - def test_get_diff__warns_and_skips_missing_column(self, mock_warn): - """Documented columns absent from the relation are warned about and skipped.""" + def test_get_diff__skips_missing_column_without_warning(self, mock_warn): + """Documented columns absent from the relation are skipped; get_diff does not warn. + + The missing-column warning now runs post-build (validate_persist_doc_columns), so the + pre-materialization diff must stay silent to avoid false-warning on a legitimately new + column. + """ reset_missing_persist_doc_column_warnings() # col2 is documented but not present in the relation config = ColumnCommentsConfig( @@ -118,19 +140,7 @@ def test_get_diff__warns_and_skips_missing_column(self, mock_warn): diff = config.get_diff(other) # Only the existing column is included in the diff; the missing one is skipped. assert diff == ColumnCommentsConfig(comments={"`col1`": "new comment"}, persist=True) - # A warning is emitted naming the missing column. - mock_warn.assert_called_once() - warned_event = mock_warn.call_args.args[0] - assert "col2" in warned_event.base_msg - assert "col1" not in warned_event.base_msg - - @patch("dbt.adapters.databricks.persist_doc_column_warnings.warn_or_error") - def test_get_diff__no_warning_when_all_present(self, mock_warn): - """No warning is emitted when every documented column exists (case-insensitively).""" - reset_missing_persist_doc_column_warnings() - config = ColumnCommentsConfig(comments={"account_id": "Account ID"}, persist=True) - other = ColumnCommentsConfig(comments={"Account_ID": ""}) - config.get_diff(other) + # No warning is emitted from the diff path. mock_warn.assert_not_called() @patch("dbt.adapters.databricks.persist_doc_column_warnings.warn_or_error") diff --git a/tests/unit/test_adapter.py b/tests/unit/test_adapter.py index 44b5af4fd..f8c8d193e 100644 --- a/tests/unit/test_adapter.py +++ b/tests/unit/test_adapter.py @@ -1212,6 +1212,45 @@ def test_get_persist_doc_columns_no_warning_when_all_present(self, mock_warn, ad adapter.get_persist_doc_columns(existing, column_dict) mock_warn.assert_not_called() + @patch("dbt.adapters.databricks.persist_doc_column_warnings.warn_or_error") + def test_validate_persist_doc_columns_warns_on_missing_column(self, mock_warn, adapter): + """A documented column absent from the (post-build) relation is warned about.""" + reset_missing_persist_doc_column_warnings() + existing = [self.create_column("col1", "comment1")] + column_dict = { + "col1": {"name": "col1", "description": "comment1"}, + "col2": {"name": "col2", "description": "typo / stale doc"}, + } + assert adapter.validate_persist_doc_columns(existing, column_dict) is None + mock_warn.assert_called_once() + warned_event = mock_warn.call_args.args[0] + assert "col2" in warned_event.base_msg + assert "col1" not in warned_event.base_msg + + @patch("dbt.adapters.databricks.persist_doc_column_warnings.warn_or_error") + def test_validate_persist_doc_columns_no_warning_for_newly_added_column( + self, mock_warn, adapter + ): + """A legitimately new column is present in the freshly built relation post-build, so the + post-build check does not false-warn about it (the E1 regression).""" + reset_missing_persist_doc_column_warnings() + # col2 was just added to the model; post-build it exists in the relation. + existing = [self.create_column("col1", "c1"), self.create_column("col2", "c2")] + column_dict = { + "col1": {"name": "col1", "description": "c1"}, + "col2": {"name": "col2", "description": "c2"}, + } + adapter.validate_persist_doc_columns(existing, column_dict) + mock_warn.assert_not_called() + + @patch("dbt.adapters.databricks.persist_doc_column_warnings.warn_or_error") + def test_validate_persist_doc_columns_case_insensitive(self, mock_warn, adapter): + reset_missing_persist_doc_column_warnings() + existing = [self.create_column("Account_ID", "")] + column_dict = {"account_id": {"name": "account_id", "description": "Account ID"}} + adapter.validate_persist_doc_columns(existing, column_dict) + mock_warn.assert_not_called() + class TestGetColumnsByDbrVersion(DatabricksAdapterBase): @pytest.fixture diff --git a/tests/unit/test_persist_doc_column_warnings.py b/tests/unit/test_persist_doc_column_warnings.py index 523a3cdfa..eeee4aa23 100644 --- a/tests/unit/test_persist_doc_column_warnings.py +++ b/tests/unit/test_persist_doc_column_warnings.py @@ -21,11 +21,15 @@ def test_warns_once_for_same_missing_set(self, mock_warn): mock_warn.assert_called_once() @patch("dbt.adapters.databricks.persist_doc_column_warnings.warn_or_error") - def test_get_diff_and_helper_share_dedupe(self, mock_warn): - """get_diff then the shared helper must not double-fire for the same cols.""" + def test_get_diff_does_not_warn_helper_still_dedupes(self, mock_warn): + """get_diff is silent (warning moved post-build); the shared helper still dedupes.""" config = ColumnCommentsConfig(comments={"col1": "new", "col2": "missing"}, persist=True) other = ColumnCommentsConfig(comments={"col1": "old"}) config.get_diff(other) + # The pre-materialization diff no longer contributes a warning. + mock_warn.assert_not_called() + # The post-build path warns once per unique set. + warn_missing_persist_doc_columns(["col2"]) warn_missing_persist_doc_columns(["col2"]) mock_warn.assert_called_once() assert "col2" in mock_warn.call_args.args[0].base_msg From ff94a2ca427f5bbc92c41fb5dad30c6d61ce7f28 Mon Sep 17 00:00:00 2001 From: Rashi Jaiswal Date: Tue, 28 Jul 2026 12:33:46 +0530 Subject: [PATCH 9/9] Trim what-comments at persist_docs validation call sites --- .../macros/materializations/incremental/incremental.sql | 4 ++-- dbt/include/databricks/macros/materializations/table.sql | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/dbt/include/databricks/macros/materializations/incremental/incremental.sql b/dbt/include/databricks/macros/materializations/incremental/incremental.sql index 5339a723c..b9f40b0bc 100644 --- a/dbt/include/databricks/macros/materializations/incremental/incremental.sql +++ b/dbt/include/databricks/macros/materializations/incremental/incremental.sql @@ -81,8 +81,8 @@ {%- endif -%} {%- endif -%} - {#-- Warn (post-build) about documented columns absent from the final relation. Runs on every - sub-branch above and regardless of incremental_apply_config_changes. --#} + {#-- Placed here so it runs on every sub-branch above (create/replace/merge) and regardless of + incremental_apply_config_changes. --#} {% do validate_persist_doc_columns(target_relation, model) %} {% set should_revoke = should_revoke(existing_relation, full_refresh_mode) %} diff --git a/dbt/include/databricks/macros/materializations/table.sql b/dbt/include/databricks/macros/materializations/table.sql index 32a411681..5247ba4a0 100644 --- a/dbt/include/databricks/macros/materializations/table.sql +++ b/dbt/include/databricks/macros/materializations/table.sql @@ -32,7 +32,6 @@ {% endif %} {% endif %} - {#-- Warn (post-build) about documented columns absent from the final relation. --#} {% do validate_persist_doc_columns(target_relation, model) %} {% set should_revoke = should_revoke(existing_relation, full_refresh_mode=True) %}