Skip to content

Commit a248840

Browse files
committed
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 databricks#1563.
1 parent 62c176d commit a248840

2 files changed

Lines changed: 56 additions & 1 deletion

File tree

dbt/adapters/databricks/relation_configs/column_comments.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
from typing import ClassVar, Optional
22

33
from dbt.adapters.contracts.relation import RelationConfig
4+
from dbt.adapters.events.types import AdapterEventWarning
45
from dbt.adapters.relation_configs.config_base import RelationResults
6+
from dbt_common.events.functions import warn_or_error
57

68
from dbt.adapters.databricks.logging import logger
79
from dbt.adapters.databricks.relation_configs.base import (
@@ -23,8 +25,28 @@ def get_diff(self, other: "ColumnCommentsConfig") -> Optional["ColumnCommentsCon
2325
# Create a case-insensitive lookup for other's column comments
2426
other_comments_lower = {k.lower(): v for k, v in other.comments.items()}
2527

28+
# Warn about columns that are documented in the model's schema but are not present in
29+
# the relation. These are skipped below (rather than erroring on the alter), so surface
30+
# them to the user to catch typos and stale documentation.
31+
missing = [
32+
column_name
33+
for column_name in self.comments
34+
if column_name.lower() not in other_comments_lower
35+
]
36+
if missing:
37+
warn_or_error(
38+
AdapterEventWarning(
39+
base_msg=(
40+
"The following columns are specified in the schema but are not present "
41+
"in the database and will be skipped: " + ", ".join(missing)
42+
)
43+
)
44+
)
45+
2646
for column_name, comment in self.comments.items():
2747
# Use case-insensitive comparison for column names
48+
if column_name.lower() not in other_comments_lower:
49+
continue
2850
other_comment = other_comments_lower.get(column_name.lower())
2951
if comment != other_comment:
3052
column_name = f"`{column_name}`"

tests/unit/relation_configs/test_column_comments_config.py

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
from unittest.mock import Mock
1+
from unittest.mock import Mock, patch
22

33
from agate import Table
44

@@ -102,3 +102,36 @@ def test_get_diff__case_mismatch_with_actual_changes(self):
102102
assert diff == ColumnCommentsConfig(
103103
comments={"`account_id`": "New Account ID"}, persist=True
104104
)
105+
106+
@patch("dbt.adapters.databricks.relation_configs.column_comments.warn_or_error")
107+
def test_get_diff__warns_and_skips_missing_column(self, mock_warn):
108+
"""Documented columns absent from the relation are warned about and skipped."""
109+
# col2 is documented but not present in the relation
110+
config = ColumnCommentsConfig(
111+
comments={"col1": "new comment", "col2": "comment for missing column"}, persist=True
112+
)
113+
other = ColumnCommentsConfig(comments={"col1": "old comment"})
114+
diff = config.get_diff(other)
115+
# Only the existing column is included in the diff; the missing one is skipped.
116+
assert diff == ColumnCommentsConfig(comments={"`col1`": "new comment"}, persist=True)
117+
# A warning is emitted naming the missing column.
118+
mock_warn.assert_called_once()
119+
warned_event = mock_warn.call_args.args[0]
120+
assert "col2" in warned_event.base_msg
121+
assert "col1" not in warned_event.base_msg
122+
123+
@patch("dbt.adapters.databricks.relation_configs.column_comments.warn_or_error")
124+
def test_get_diff__no_warning_when_all_present(self, mock_warn):
125+
"""No warning is emitted when every documented column exists (case-insensitively)."""
126+
config = ColumnCommentsConfig(comments={"account_id": "Account ID"}, persist=True)
127+
other = ColumnCommentsConfig(comments={"Account_ID": ""})
128+
config.get_diff(other)
129+
mock_warn.assert_not_called()
130+
131+
@patch("dbt.adapters.databricks.relation_configs.column_comments.warn_or_error")
132+
def test_get_diff__no_warning_when_not_persisting(self, mock_warn):
133+
"""Missing columns are not evaluated (or warned about) when persist is False."""
134+
config = ColumnCommentsConfig(comments={"col1": "comment", "col2": "comment"})
135+
other = ColumnCommentsConfig(comments={"col1": "comment"})
136+
assert config.get_diff(other) is None
137+
mock_warn.assert_not_called()

0 commit comments

Comments
 (0)