fix: warn on missing persist_docs columns for view/materialized_view/streaming_table create - #1615
Open
r-jais wants to merge 17 commits into
Open
fix: warn on missing persist_docs columns for view/materialized_view/streaming_table create#1615r-jais wants to merge 17 commits into
r-jais wants to merge 17 commits into
Conversation
### Description Removes internal logging-event classes that have had **no call sites** since the cursor-management (databricks#910/databricks#912) and pipeline (databricks#849) refactors. They are dead code: zero consumers across source and tests, no `events/__init__.py` re-export, and no dynamic/string references. Git history confirms they were once used and their consumers were later deleted (orphaned, not never-wired). Removed: - `events/credential_events.py` — whole module (`CredentialLoadError`, `CredentialSaveError`, `CredentialShardEvent`) - `events/pipeline_events.py` — whole module (`PipelineEvent`, `PipelineRefresh`, `PipelineRefreshError`) - `events/connection_events.py` — `ConnectionReset`, `ConnectionReuse`, `ConnectionIdleClose`, `ConnectionCreated` Kept (still live or still a needed base): `ConnectionEvent`, `ConnectionCreateError`, the `ConnectionWrapperEvent` base, `ConnectionCreate`, and all of `events/base.py` / `events/other_events.py` / `logging.py`. Targeting `1.13.latest` rather than a patch: although these were never public API, they sit at importable paths, so the removal is scoped to the next minor as a safety margin. Full unit suite passes (1081 passed, 6 skipped); ruff/ruff-format/mypy clean. ### Checklist - [x] I have run this code in development and it appears to resolve the stated issue - [x] This PR includes tests, or tests are not required/relevant for this PR - [x] I have updated the `CHANGELOG.md` and added information about my change to the "dbt-databricks next" section.
…_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).
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.
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
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.
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.
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.
…persist_docs.columns
…ming_table create
r-jais
force-pushed
the
fix/persist-docs-warn-missing-create-paths
branch
from
August 6, 2026 09:06
31870e8 to
f1b69e6
Compare
Collaborator
|
Can we rebase on the latest code for #1563 ? |
sd-db
added
the
pending on pr-author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
resolves #1399
Follow-up to #1563. That PR added the "warn when a documented column is absent from the relation" check as a post-build validation (
validate_persist_doc_columns, mirroring the sharedvalidate_doc_columnsbehavior every other adapter uses) and wired it into the table and incremental materializations.This PR completes the create-time coverage by calling the same gated, post-build validation on the materializations #1563 didn't touch:
dbt/include/databricks/macros/materializations/view.sql.../materialized_view.sql.../streaming_table.sqlEach is a single
validate_persist_doc_columns(target_relation, model)call after the relation is built. The macro is gated onconfig.persist_column_docs()and applies no comments, so it never fires when column persistence is off and stays safe under--warn-error(per the caution in #1563 about not warning unconditionally inparse_columns_and_constraints). Previously these paths iterated only the query's output columns (get_persist_docs_column_list) or built inline comments viaparse_columns_and_constraints, so a YAML-only documented column was silently dropped.Stacking
Tests
Functional (
tests/functional/adapter/persist_docs/test_persist_docs.py): create-time missing-column warning for view, materialized view, and streaming table (each: documented column absent from the relation → warns exactly once, gated onpersist_docs.columns). New fixtures intests/functional/adapter/persist_docs/fixtures.py. The corevalidate_persist_doc_columnslogic is unit-tested in #1563.Checklist
CHANGELOG.mdand added information about my change to the "dbt-databricks next" section.