feat: Report rows affected in Databricks adapter responses - #1632
feat: Report rows affected in Databricks adapter responses#1632cjcdoomed wants to merge 54 commits into
Conversation
## Description The committed `uv.lock` recorded an internal Databricks PyPI proxy (`https://pypi-proxy.dev.databricks.com/simple/`) on all 116 `source = { registry = ... }` lines. This rewrites those lines to uv's canonical public default, `https://pypi.org/simple`. Only the `source.registry` field changes. Package versions, download URLs (`files.pythonhosted.org`), and hashes are byte-identical — the diff is 116 registry lines and nothing else. ## Checklist - [x] Only `uv.lock` `source.registry` values changed; hashes and download URLs untouched - [x] Lockfile still parses as valid TOML (113 packages) - [x] New string matches uv's canonical default `https://pypi.org/simple` (no trailing slash)
## Summary - Add `.agents/` to `.gitignore` so local agent state stays untracked. ## Testing - `git check-ignore .agents/` - `hatch run pre-commit run --files .gitignore`
<!-- Please review our pull request review process in CONTRIBUTING.md
before your proceed. -->
Resolves #
<!---
Include the number of the issue addressed by this PR above if
applicable.
Example:
resolves databricks#1234
Please review our pull request review process in CONTRIBUTING.md before
your proceed.
-->
### Description
<!--- Describe the Pull Request here -->
Automates the weekly refresh of .github/test_timings.json so integration
shard balancing doesn't drift stale. Adds a scheduled Refresh Test
Timings workflow that regenerates timings from the last few distinct-SHA
green integration runs and opens a PR when timings change substantially.
### Checklist
- [x] I have run this code in development and it appears to resolve the
stated issue -> Ran it locally end-to-end against the live repo
- [ ] I have updated the `CHANGELOG.md` and added information about my
change to the "dbt-databricks next" section - NA
- [ ] [Optional] I have run `/dbt-databricks-pr-ready` (AI agent skill
in `.claude/skills/`) and addressed its merge-readiness feedback
---------
Co-authored-by: Shubham Dhal <shubham.dhal@databricks.com>
…2) (databricks#1590) Resolves # ### Description Adds `catalog_database` support for Unity catalogs in catalogs.yml v2, so a model can be routed to an explicit **physical Unity catalog** independent of the dbt catalog label (`name`). This mirrors the Fusion implementation (dbt-labs/fs#11436): `databricks__generate_database_name` now returns `catalog_relation.catalog_database` first, taking precedence over the model `database` config and `catalog_name`. Precedence (matches Fusion): `catalog_database` > model `database` config > `catalog_name` > `target.database`. **Config** (v2 catalogs.yml): ```yaml catalogs: - name: my_catalog # dbt label (models reference this via catalog_name) type: unity table_format: iceberg config: databricks: catalog_database: prod_catalog # physical Unity catalog ``` **Changes** - `DatabricksCatalogRelation`: add `catalog_database` field. - `UnityCatalogIntegration`: read the first-class `catalog_database` field (surfaced on the config by the base `CatalogIntegration` from the v2 catalogs.yml config block) and carry it onto the relation. Hive-metastore is unaffected (no catalog layer). - `databricks__generate_database_name`: prefer `catalog_database`, mirroring fs. - No dbt-adapters pin bump: `catalog_database` is read via `getattr(config, "catalog_database", None)` and the relation always carries the field, so on dbt-adapters `<1.24.4` (which has no `catalog_database` on the catalog config) it resolves to `None` — a graceful no-op, no crash. It's honored on `dbt-adapters>=1.24.4`. - Unit tests (integration + macro precedence) + CHANGELOG. No behavior change for existing projects that don't set `catalog_database`. **Requirements:** `catalog_database` takes effect only under v2 catalogs — needs **dbt-core ≥ 1.12** (`use_catalogs_v2`, which triggers `bridge_v2_catalog`) and **dbt-adapters ≥ 1.24.4** (bridge surfaces `catalog_database` onto the catalog config). Below that it resolves to `None` (graceful no-op). Note: dbt-core's own `CatalogWriteIntegrationConfig` does not carry `catalog_database` even on 1.13.0a1, so the value flows only through the dbt-adapters v2 bridge — the v1 `write_integrations` path can't carry it.⚠️ **dbt-core pin:** `pyproject.toml` currently pins `dbt-core <1.11.13`, which **excludes 1.12** — so the feature can't be exercised with the adapter's current pin. Merging as-is lands it dormant/forward-looking; activating it needs a dbt-core pin bump to allow ≥1.12 (separate/maintainer decision). **Testing:** unit-tested — the `catalog_database` config→integration→relation flow (`test_catalogs_v2.py`, incl. a graceful-degradation case for adapters without the field) and the macro precedence via a rendered `databricks__generate_database_name` (`test_generate_database_name.py`: `catalog_database` > model `database` > `catalog_name` > `target.database`). End-to-end `dbt parse` on a v2-capable dbt-core is not run here (the pin caps below 1.12). ### 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. - [ ] [Optional] I have run `/dbt-databricks-pr-ready` (AI agent skill in `.claude/skills/`) and addressed its merge-readiness feedback --------- Co-authored-by: Shubham Dhal <shubham.dhal@databricks.com>
### Description Maintainer approval for a fork pull request should authorize the exact commit that was reviewed. Previously, a contributor could push another commit after approval but before the privileged integration workflow checked out the pull request head. This change captures the pull request head SHA when the integration command is accepted, passes that immutable SHA to both integration workflows, validates it before checkout, and reports the abbreviated SHA in the pull request status comment. Manual `git_ref`, manual pull request number, scheduled, and minimum-dependency execution paths retain their existing behavior. Regression tests execute the embedded trigger and parser scripts to verify SHA capture and propagation, both workflow checkout targets, and rejection of malformed SHAs. ### 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 - [ ] I have updated the `CHANGELOG.md` and added information about my change to the "dbt-databricks next" section. (Not applicable: CI-only change with no runtime behavior impact.) - [ ] [Optional] I have run `/dbt-databricks-pr-ready` (AI agent skill in `.claude/skills/`) and addressed its merge-readiness feedback Verification: - `hatch run pytest tests/unit/test_integration_workflow_dispatch.py -v` (`5 passed`) - Full unit suite (`1284 passed, 6 skipped`) - `hatch run pre-commit run --all-files` - `git diff --check`
### Description The project PR-readiness skill previously assumed an upstream maintainer was preparing another contributor's PR. That excluded contributors checking their own changes and made fork destinations depend on checkout conventions. This change makes the workflow usable from supported coding agents while keeping writes explicit and narrowly scoped: - Stores the canonical skill and behavioral contract under `.agents/skills`, with a thin `.claude` compatibility entry point. - Classifies contributor-owned, in-repository, opted-in maintainer, and read-only PR modes from live metadata. - Resolves the exact head repository and branch, then revalidates identity, permissions, destination, and head SHA before commits and pushes. - Keeps worktrees and reports under ignored `.agents` paths and makes the PR-template wording agent-neutral. ### Checklist - [x] I have run this workflow in development and verified its decision behavior - [x] This PR includes a co-located behavioral contract covering contributor, fork, maintainer, moved-head, and approval-gate cases - [ ] I have updated the `CHANGELOG.md` — not applicable because this changes contributor workflow documentation, not adapter runtime behavior - [ ] [Optional] I have run the `dbt-databricks-pr-ready` project skill for this PR and addressed its merge-readiness feedback ### Verification - [x] `hatch run pre-commit run --all-files` - [x] Static reference, portability, and forbidden-token scans - [x] Six-case OLD/NEW decision comparison with no regressions or residuals
Resolves # ### Description A scheduled timing refresh can legitimately find no eligible integration-test artifacts while the workflow is waiting for a complete sample. Previously, that condition returned a failure and stopped the refresh workflow. Automatic discovery now exits successfully when no runs yield timing data, allowing the next scheduled refresh to retry. Explicit `--run-ids` requests still fail when they produce no timings, preserving their validation behavior. The change adds regression coverage for the automatic zero-run path. ### 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 - [ ] I have updated the `CHANGELOG.md` and added information about my change to the "dbt-databricks next" section. (not applicable: CI-only timing refresh behavior) ### Verification - `hatch run pytest tests/unit/test_regenerate_timings.py -v` - `hatch run pre-commit run --all-files`
…ropriately (databricks#1595) Resolves databricks#1594 ### Description - Ensures that in incremental strategies columns are backquoted appropriately - Change the relevant macros to use `adapter.quote` ### 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. - [x] [Optional] I have run `/dbt-databricks-pr-ready` (AI agent skill in `.claude/skills/`) and addressed its merge-readiness feedback
…ricks#1591) (databricks#1593) Resolves databricks#1591 ### Description Python models configured with `table_format='iceberg'` and the `use_managed_iceberg` behavior flag failed to materialize with: ``` INVALID_PARAMETER_VALUE.MANAGED_TABLE_FORMAT: Only Delta is supported for managed tables. Provided datasource format is PARQUET. ``` **Root cause:** `adapter.resolve_file_format` returns the `"parquet"` sentinel for managed Iceberg. That sentinel is only meaningful to the SQL path, where [`file_format_clause`](https://github.com/databricks/dbt-databricks/blob/main/dbt/include/databricks/macros/relations/file_format.sql) overrides it to `using iceberg`. The PySpark writer in `python.sql` had no such override and emitted `.format("parquet")` verbatim into the generated `saveAsTable(...)` call, which Unity Catalog rejects for managed tables. The equivalent SQL model (`table_format='iceberg'` + `use_managed_iceberg`) succeeds, and a Python model without `table_format='iceberg'` (default Delta) also succeeds — only the Python + managed-Iceberg combination failed. **Fix:** Add a `py_resolve_writer_format()` macro that mirrors the SQL override — it returns `'iceberg'` when `table_format='iceberg'` and `use_managed_iceberg` is enabled, and otherwise defers to `resolve_file_format` (unchanged behavior for delta and all other formats). It is used in both writer paths: `py_get_writer_options` (non-incremental / `table`) and `create_python_intermediate_table` (incremental staging). `resolve_file_format` itself is intentionally left unchanged, so the 15 SQL-path consumers of the sentinel are unaffected. ### Verification Verified end-to-end against a **DBR 18.2** all-purpose cluster on Unity Catalog (dbt-core 1.11.12, dbt-databricks from this branch): | Scenario | Before | After | | --- | --- | --- | | SQL model, managed Iceberg (control) | ✅ pass | ✅ pass (unchanged) | | Python `table` model, managed Iceberg | ❌ `MANAGED_TABLE_FORMAT` | ✅ creates MANAGED table, UniForm Iceberg enabled | | Python `incremental` model — initial build | ❌ | ✅ | | Python `incremental` model — merge (staging table) | ❌ | ✅ `.format("iceberg")` for `__dbt_tmp`, merge succeeds | Confirmed the resulting table is `table_type=MANAGED` with `delta.enablemanagedicebergtable=true` and `delta.universalFormat.enabledFormats=iceberg` — matching the working SQL path. Unit tests: `tests/unit/macros/adapters/test_python_macros.py` (2 new cases: managed-iceberg → `iceberg`, iceberg-without-flag → `delta`); full macro suite (237) passes. ### 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. - [ ] [Optional] I have run `/dbt-databricks-pr-ready` (AI agent skill in `.claude/skills/`) and addressed its merge-readiness feedback This pull request and its description were written by Isaac. --------- Signed-off-by: Divya Siruvuri <divyasiruvuri@gmail.com> Co-authored-by: Shubham Dhal <shubham.dhal@databricks.com>
…lds (databricks#1592) <!-- Please review our pull request review process in CONTRIBUTING.md before your proceed. --> Resolves databricks#1165 <!--- Include the number of the issue addressed by this PR above if applicable. Example: resolves databricks#1234 Please review our pull request review process in CONTRIBUTING.md before your proceed. --> ### Description <!--- Describe the Pull Request here --> Shallow clones have a distinct table_type that can't be changed in place, so `dbt clone` and table/incremental rebuilds over a clone failed (`UPDATE_TABLE_TYPE`, `CLUSTER_BY_AUTO_UNSUPPORTED_TABLE_TYPE_ERROR`). Adds `is_shallow_clone`, drops the clone before recreating it (clone/table/incremental), and skips `CLUSTER BY AUTO` when altering a clone in place. ### Checklist - [ ] 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. - [ ] [Optional] I have run `/dbt-databricks-pr-ready` (AI agent skill in `.claude/skills/`) and addressed its merge-readiness feedback --------- Co-authored-by: Shubham Dhal <shubham.dhal@databricks.com>
Resolves # ### Description `dbt-databricks` no longer uses `keyring`; token caching moved to the Databricks SDK, but the direct dependency and its old lock entries remained. This removes the unused direct dependency, stale transitive packages from both lockfiles, and skipped tests that only exercised the retired keyring-backed implementation. There is no runtime authentication behavior change. ### Checklist - [x] `hatch run pytest tests/unit/test_auth.py -q -n 0 --profile databricks_cluster` — 19 passed, 2 skipped - [x] `hatch run code-quality` — passed, including `uv-lock` and the public-PyPI URL check - [x] Validated the edited `uv.lock` graph — 103 packages with no missing dependency references - [x] I have updated the `CHANGELOG.md` and added information about my change to the "dbt-databricks next" section.
## Description Adds `eval-spec.md` alongside the `dbt-databricks-pr-ready` skill's `SKILL.md`. It holds the skill's **equivalence rubric** (decision dimensions) and its **branch-coverage corpus** (cases p1-p4). Co-locating the eval-spec keeps the equivalence contract next to the skill it measures. **Docs only — no runtime impact.** No CHANGELOG entry (consistent with databricks#1575, which added the skill itself without one). ## Checklist - [x] Docs-only change under `.claude/skills/` (no adapter/runtime code touched) - [x] No CHANGELOG entry required (not runtime-affecting; matches the databricks#1575 precedent) - [x] No tests required (documentation file)
…bricks#1456) fixes databricks#1404 ### description when `view_update_via_alter: true` and `use_materialization_v2: true` are enabled, `dbt run --full-refresh` on an unchanged view produces a no-op instead of forcing recreation. ### changes added a guard clause to `relation_should_be_altered()` to return `False` when `should_full_refresh()` is true ensuring `replace_with_view()` is called unconditionally, also added relevant tests. ### Checklist - [x] I have run this code in development and it appears to resolve the stated issue - [x] This PR includes tests for the fix - [x] I have updated the `CHANGELOG.md` and added information about my change --------- Co-authored-by: Shubham Dhal <shubham.dhal@databricks.com>
<!-- Please review our pull request review process in CONTRIBUTING.md before your proceed. --> Resolves databricks#1323 <!--- Include the number of the issue addressed by this PR above if applicable. Example: resolves databricks#1234 Please review our pull request review process in CONTRIBUTING.md before your proceed. --> ### Description <!--- Describe the Pull Request here --> Dropping a column that carries a governed tag fails with CANNOT_DROP_TAGGED_COLUMN, because Unity Catalog requires the tag be removed before the column. This unsets any tags on the columns being dropped (via on_schema_change='sync_all_columns') before issuing DROP COLUMNS, so tagged columns can be dropped cleanly. ### 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. - [ ] [Optional] I have run `/dbt-databricks-pr-ready` (AI agent skill in `.claude/skills/`) and addressed its merge-readiness feedback
### Description Raise the `dbt-core` ceiling to `<1.12.1` so dbt-databricks accepts dbt-core 1.12.0 while keeping later 1.12 patch upgrades intentional. The lock now pins the adapter test environment to dbt-core 1.12.0 and its required dependency graph. ### 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. ## Test Plan - [x] Full current-dependency unit suite - [x] Minimum-direct-dependency parse and unit suites - [x] Live SQL warehouse basic materialization E2E test - [x] Pre-commit and lock consistency checks ## Verification Commands ```bash hatch run unit hatch run min-deps:parse hatch run min-deps:unit hatch run pytest --color=yes -v --profile databricks_uc_sql_endpoint tests/functional/adapter/basic/test_base.py ```
### Description Hive Metastore relation listings do not report Databricks-specific table types. As a result, `dbt clone --full-refresh` treated an existing HMS shallow clone as a regular table, dropped it, and recreated it at Delta version 0 instead of replacing it in place. This keeps targets with unknown HMS table types on the in-place `CREATE OR REPLACE ... SHALLOW CLONE` path. Known Unity Catalog managed/external tables and non-table relations still use the drop-first path required to avoid `UPDATE_TABLE_TYPE` errors. ### 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 - [ ] [Optional] I have run the `dbt-databricks-pr-ready` project skill for this PR and addressed its merge-readiness feedback ### Test Plan - [x] Added unit coverage for HMS relations with unknown table type - [x] Ran `TestCloneRequiresDrop` (5 passed) - [x] Ran the HMS `TestCloneShallowClone::test_shallow_clone` functional test (1 passed) - [x] Completed an independent adversarial review with no required fixes ### Verification Commands ```bash hatch run pytest tests/unit/macros/materializations/test_clone_macros.py::TestCloneRequiresDrop -v hatch run pytest tests/functional/adapter/dbt_clone/test_dbt_clone.py::TestCloneShallowClone::test_shallow_clone -v --profile databricks_cluster ```
Resolves databricks#1601 ### Description Streaming Tables with configured databricks_tags did not fetch their existing table tags during relation metadata planning. On every unchanged rerun, the configured tags were compared with an empty server state and falsely reported as a configuration change. This had broader effects than the fail case alone: - With the default apply behavior, the false change entered the alter path, issued an unnecessary CREATE OR REFRESH STREAMING TABLE, and set the configured tags. The common execution wrapper then applied the tags again. - With fail, the unchanged run aborted. - With continue, the unchanged run warned and skipped execution. This change adds the same conditional tag fetch already used by Materialized Views: - Fetch existing table tags when configured tags require server metadata. - Skip the Information Schema query when no tags are configured. - Preserve the safe fetch when model_config is absent. Identical tags now produce no tag changeset, so Streaming Tables follow their normal no-change refresh policy. Manual and cron refresh behavior remains unchanged, while auto-refreshed EVERY and ON UPDATE modes can remain true no-ops. The change is limited to table-level tag metadata planning. Column tags, tag removal, and duplicate tag writes for genuine tag changes remain unchanged. ### Verification - [x] Reproduced the unchanged second-run false change before the fix using on_configuration_change set to fail. - [x] Added metadata-planning unit coverage for tagged, untagged, and missing-config Streaming Tables. - [x] Verified unchanged and updated Streaming Table tags and existing Materialized View tag behavior against Databricks. - [x] Ran the full unit suite: 1,314 passed and 5 skipped. - [x] Ran all repository pre-commit checks. ### 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. - [ ] [Optional] I have run the dbt-databricks-pr-ready project skill for this PR and addressed its merge-readiness feedback
### Description Concurrent connection opens can reach lazy Databricks SDK configuration initialization at the same time. The SDK performs first-use class metadata initialization, so overlapping constructors can fail with `RuntimeError: dictionary changed size during iteration`. Serialize adapter-managed cold initialization with a module-level lock while retaining the unlocked cached fast path. A second cache check inside the lock ensures one SDK `Config` is constructed and shared. Credential construction remains lazy, preserving offline parse, list, and compile behavior. The regression test uses barriers to force eight threads past the initial cache check and verifies one constructor call and shared result identity. ### Verification - `hatch run unit`: 1,313 passed, 4 skipped - `hatch run code-quality`: passed - SQL warehouse concurrent schema-listing stress test: 2 passed ### 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. - [ ] [Optional] I have run the `dbt-databricks-pr-ready` project skill for this PR and addressed its merge-readiness feedback
…cks#1616) ## Summary - Skip UC profiles for `TestCloneHmsRelationMatrix` so CI does not fail when a known managed target is drop-first replaced (follow-up to databricks#1608 review finding I1). - Share a module-level `_delta_history` helper and assert prior Delta versions survive in-place full-refresh clone (stronger history-survival check). - Add unit coverage for `databricks_table_type="external"` requiring drop. ## Test plan - [x] `hatch run unit tests/unit/macros/materializations/test_clone_macros.py::TestCloneRequiresDrop -v` - [x] `hatch run pytest tests/functional/adapter/dbt_clone/test_dbt_clone.py::TestCloneHmsRelationMatrix::test_clone_full_refresh_over_hms_relation_shapes tests/functional/adapter/dbt_clone/test_dbt_clone.py::TestCloneShallowClone::test_shallow_clone -v --profile databricks_cluster`
### Description Prepares dbt-databricks 1.12.3 for release. - Sets the release date to July 29, 2026. - Adds missing community credits while preserving the reviewed changelog wording and existing PR/issue links. - Bumps the adapter version from 1.12.2 to 1.12.3. ### Verification - `hatch run code-quality` - `hatch run unit` (1,318 passed, 4 skipped) - `hatch run pre-commit run --all-files` ### 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 - [ ] [Optional] I have run the `dbt-databricks-pr-ready` project skill for this PR and addressed its merge-readiness feedback
…ss reruns (databricks#1618) ## Summary - Raise the `pytest-rerunfailures` lower bound to `>=16.2` (and regenerate the min-deps lock) so CI no longer pins 14.0 - In 14.0, a failed test's rerun teardown dropped class-scoped setup finalizers, leaking the first class's dbt `project` fixture into later classes on the same xdist worker - That cascade turned a real `TestSpecifyingHttpPath` cluster REPL failure into false failures in `TestJobClusterMissingConfig`, `TestAllPurposeClusterMissingClusterId`, and `TestManagedIcebergPythonModel` on Min-Deps SQL-warehouse shard 0 ([run 30367154216](https://github.com/databricks/dbt-databricks/actions/runs/30367154216/job/90368831106)) - Remove the `SchemaNameVarMixin` workaround that was papering over the same leak (test-only, no runtime impact) ## Test plan - [x] Minimal repro: class-scoped fixture leak on `pytest-rerunfailures==14.0` with `--reruns 1`; absent on 15.0–16.2 - [x] Functional: deliberate rerun trigger + missing-config tests fail under 14.0, pass under 16.2 (SQL warehouse profile) - [x] Functional: de-mixined `TestChangingSchema` / `TestChangingSchemaIncremental` pass on SQL warehouse - [x] `hatch run code-quality` / pre-commit hooks pass - [ ] CI Min-Deps Integration Tests green (or only the known flaky `TestSpecifyingHttpPath` REPL failure, without the cascade)
### Description Add concise contributor guidance that defaults to no code comment unless it preserves a non-obvious constraint, workaround, or rationale. The policy discourages comments that narrate code, repeat tests, preserve implementation history, or duplicate pull request and changelog context. ### Checklist - [x] Tests are not required for this contributor-guidance-only change - [x] `CHANGELOG.md` is unchanged because this does not affect runtime behavior
…rity) (databricks#1623) ## Summary Fixes the following CVE's being flagged in this project ``` - CVE-2026-41603 — TLS cert validation bypass, fixed in 0.23.0 - CVE-2026-43868 — Memory allocation with excessive size, fixed in 0.23.0 - CVE-2026-48586 — Data amplification/DoS (highly compressed data), fixed in 0.24.0 ``` Raises the `databricks-sql-connector` upper bound from `<4.3.1` to `<4.5.0` and pins the connector at **4.4.0** in `uv.lock`. `databricks-sql-connector==4.4.0` requires `thrift>=0.24.0`, which addresses a known vulnerability in older thrift releases. The previous ceiling held the connector at `4.3.0`, which pulled `thrift==0.22.0` which is the vulnerable version. ## Changes | File | Change | |---|---| | `pyproject.toml` | `databricks-sql-connector[pyarrow]` ceiling: `<4.3.1` → `<4.5.0` | | `uv.lock` | Connector pinned to `4.4.0`; `thrift` bumped `0.22.0` → `0.24.0` | ## Testing All 1318 unit tests pass with no regressions locally ## Changelog ### Under the Hood - Bump `databricks-sql-connector` to 4.4.0, pulling in `thrift` 0.24.0 (security fix) --------- Co-authored-by: Shubham Dhal <shubham.dhal@databricks.com>
…icks#1613) Resolves databricks#1251 ### Description `get_create_sql_comment` interpolates a model description straight into a SQL string literal without escaping, so a description containing an apostrophe produces an unterminated literal and the statement fails to parse: ``` COMMENT 'Bob's model' ``` Every other place the adapter emits a comment already escapes: `relations/comment.sql`, `adapters/persist_docs.sql` (three sites), `relations/view/create.sql` and `materializations/seeds/helpers.sql`. This one was missed, so it is the only raw interpolation left in the macro tree. The macro is reached from three call sites, all of which pass a relation-config value straight through: `relations/materialized_view/create.sql`, `relations/streaming_table/create.sql` and `relations/streaming_table/alter.sql`. So a materialized view or streaming table with `persist_docs: {relation: true}` and an apostrophe in its description cannot be built. **Fix:** apply the same `replace` filter the sibling macros already use. The escaping happens only at render time, and `CommentProcessor` continues to compare the raw project value against the raw server value, so this introduces no configuration drift on reruns. **Scope note:** I kept the fix identical to the existing convention rather than centralising the filter into a shared helper. Consolidating the six sites is a reasonable follow-up but would turn a one-line correctness fix into a six-file refactor. **Testing:** four macro tests cover a plain comment, one apostrophe, several apostrophes, and the `none` case that keeps the existing `comment is string` guard covered. The two escaping tests fail before the change and pass after. For server-observable proof, `materialized_view_schema` now carries a model description containing an apostrophe and `test_create_materialized_view_with_comment_and_constraints` asserts the comment round-trips through `information_schema.tables`; that test errors before this fix. ### 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. --------- Signed-off-by: Sreerama Yeshwanth Gowd <yeshwanthgowdsreerama@gmail.com> Co-authored-by: Shubham Dhal <shubham.dhal@databricks.com>
Resolves databricks#1611 ### Description On the `delete+insert` strategy with a composite `unique_key`, the legacy DBR < 17.1 path builds one `IN (SELECT ...)` per key column and ANDs them, so the DELETE predicate is the cross product of the key columns' value sets rather than the set of key tuples. Rows whose key tuple is absent from the source are deleted and never re-inserted, silently. With `unique_key: ['a', 'b']`, a target holding `(1,10) (2,20) (1,20) (2,10)` and a source producing only `(1,10) (2,20)`, the predicate `a IN (1,2) AND b IN (10,20)` matches all four rows. `(1,20)` and `(2,10)` are lost. The DBR 17.1+ branch of the same macro is already row-wise, and dbt-core's cross-adapter default is `where (unique_key_str) in (select distinct unique_key_str from source)`, so today the same model with the same config produces different data depending on runtime version. **Fix:** use a correlated `EXISTS` predicate that compares every key column against the same source row with null-safe equality. ```sql delete from target where exists ( select 1 from source where target.`a` <=> source.`a` and target.`b` <=> source.`b` ) ``` The single-key branch remains byte-for-byte identical, so its behavior is unchanged and remains covered by the existing single-key and non-ASCII tests. **Note on the changed tests:** `test_delete_insert_legacy_sql__multiple_unique_keys` previously asserted the per-column form, so it encoded the bug and had to be updated. Two neighbouring tests that asserted against their own hardcoded strings are converted to real macro renders, following your request on databricks#1595. **Testing:** - 17 focused unit tests cover composite keys, incremental predicates, and the unchanged single-key path. - A functional regression forces the legacy `DELETE` + `INSERT` branch on live Databricks and verifies exact replacements, crossed-tuple preservation, and absent-row preservation. - The correlated `DELETE` statement was also run directly on DBR 16.4 LTS and produced the expected final rows. A full dbt end-to-end run on compute below DBR 17.1 would add release-matrix confidence, but the changed predicate and complete dbt execution path are covered by the complementary checks above. ### 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. --------- Signed-off-by: Sreerama Yeshwanth Gowd <yeshwanthgowdsreerama@gmail.com> Co-authored-by: Shubham Dhal <shubham.dhal@databricks.com>
Resolves databricks#1609 ### Description `redact_credentials` is applied to every logged statement (`connections.py:244` and `:342`), but the `COPY INTO` pattern behind it misses several shapes, so secrets can reach `logs/dbt.log` verbatim: an uppercase `CREDENTIAL (...)` clause, the `encryption (...)` clause that `databricks_copy_into` emits itself, a dotted key such as `'fs.azure.account.key'`, a value containing a newline, and every clause after the first. The same function has two problems that are not leaks. Splitting the clause body on `,` raises `ValueError: not enough values to unpack` when a value contains a comma, and because the call sits on line 244 before the `try:` on line 246, the exception escapes while the context manager is being entered, so the statement never runs. Separately, `'.*?'` inside a repeated group backtracks exponentially on an unterminated clause. **Fix:** match `credential` and `encryption` case-insensitively, recover the keys with a regex instead of splitting on `,`, and `sub` every clause. A quoted value is described as `'(?:[^']|'(?!\s*[,)]))*'`, which treats a quote followed by a delimiter as the closing quote and any other quote as part of the value. That keeps the match linear and still redacts values containing a quote. A cheap substring test runs first so the common no-clause path does not pay for the case-insensitive scan; a 36 KB statement with no clause ends up faster than before, and the pathological input drops from roughly 24 s to sub-millisecond. The keyword deliberately has no `\b` prefix. The current lookbehind has no boundary requirement, so `storage_credential (...)` is redacted today, and adding one would silently narrow coverage. Key names are preserved and every value in a matched clause is redacted, including a non-secret one such as `'TYPE' = 'AWS_SSE_C'`, which matches the existing behaviour for `credential (...)`. **Scope note:** this repairs the existing `COPY INTO` redaction only. Other syntax that can carry secrets, such as the `OPTIONS (...)` clause, is out of scope here. **Testing:** `1321 passed, 5 skipped` on the unit suite. The three existing redaction tests are unchanged and still pass byte for byte, which confirms no behaviour regression. Ten cases are added, one per shape above plus negative cases proving that `credential_id = 1` and a clause with no `'key' = 'value'` options are left alone. No functional test is included because log redaction is not observable against a warehouse. ### 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. --------- Signed-off-by: Sreerama Yeshwanth Gowd <yeshwanthgowdsreerama@gmail.com> Co-authored-by: Shubham Dhal <shubham.dhal@databricks.com>
…ricks#1625) ## Summary Gate new `@available` methods behind portable alternatives in the agent guide. Each `@available` method is permanent public API — Jinja is late-bound, so nothing flags an unused one — and must be re-implemented in the Fusion engine's Rust adapter before behavior matches across engines. Shared-contract macros get that for free. `### Adding New Adapter Method` now opens with that cost, then lists options to work down, stopping at the first that works: a shared-contract macro, plain Jinja, a relation config component, and only then a new `@available` method. Reaching the last step asks for a justification, portable types, and an acknowledgement that Fusion parity is owed. Shorter entries added to `### Code Patterns to Follow` and `### Common Pitfalls for Agents`. ## Test plan Docs only — no runtime change, no CHANGELOG entry, no tests. `pre-commit run --all-files` passes.
## Description The unit test job in `main.yml` runs the suite with coverage instrumentation (`test:unit-with-cov`), which is slower than the other unit jobs and can exceed the shared 20-minute timeout. This raises the timeout for that job to 30 minutes, leaving the other unit jobs (including `main-min-deps.yml`) at 20. Example timeout: https://github.com/databricks/dbt-databricks/actions/runs/31261109055/job/93111926509?pr=1621 ## Checklist - [x] CI-only change; no runtime behavior change, so no CHANGELOG entry required.
…cros (databricks#1627) ## Description Reorganizes `docs/` and brings the materialization flow diagrams back in sync with the current macros. ### Structure - Split `docs/` into **`docs/flow/`** (architecture — the materialization flow diagrams) and **`docs/guides/`** (user-facing how-tos), keeping `dbt-databricks-dev.md`, `testing.md`, and `dbr-capability-system.md` at the top level. Moves preserve history. - New **`docs/README.md`** (index) and **`docs/flow/README.md`**, which explains the `use_materialization_v2` behavior flag (defaults to `False`) that selects between the V1 and V2 diagrams. - New flow docs for **snapshot**, **streaming table**, and **materialized view** (previously undocumented). - Every doc now carries a `_Last updated:_` stamp. ### Flow-diagram sync (targeted fixes, verified against the macros) - **seed V2**: removed the transaction split / `COMMIT` / index creation that `create_seed_v2` no longer performs; split the view/MV vs streaming-table compiler errors. **seed V1**: gated index creation on first-run/full-refresh (`seeds.sql`). - **table V2** and **incremental "New"**: removed the fabricated up-front staging-cleanup node. - **incremental "New"**: reordered the config-change steps to match `apply_config_changeset` (tags → tblproperties → liquid → comments → constraints). - **incremental "Existing"**: gated liquid clustering inside the config-change branch, matching the macro. - `table V1`, `view V1/V2`, `replace`, `snapshot`, `streaming_table`, `materialized_view`: verified accurate, unchanged. ### Docs wiring & drift fixes - Wired the flow docs into `AGENTS.md` with a "keep docs in sync, **code is the source of truth**" note. - Fixed the broken `README.md` link to `databricks-workflows.md` (+ moved guide paths), updated `CONTRIBUTING.MD` `tox` → `hatch` commands, and reconciled the Python `3.9` → `3.10` floor. ## Testing Docs-only — no runtime impact. All 8 flow docs' Mermaid diagrams render via `mermaid-cli` (13 charts); internal links resolve; `pre-commit run --all-files` passes.
|
@cjcdoomed I added focused functional repros for the remaining response-ownership cases here: diff. All four tests confirm the warehouse state first, then fail because the result returns plain OK without rows_affected.
|
| {%- else %} | ||
| {%- for sql in statements %} | ||
| {% call statement(name="main") %} | ||
| {% call statement(name="main" if loop.first else "statement_" ~ loop.index) %} |
There was a problem hiding this comment.
main is not just a label here: dbt serializes it as the model’s adapter response. Making loop.first the main result means list order now defines the user-visible rows_affected. For delete+insert, DELETE wins and INSERT is stored as statement_2.
That is a behavior change, not only a naming choice. Please define which count is authoritative, or aggregate the counts, and use names that describe the statements rather than main / statement_N. The same positional rule is duplicated in both incremental paths.
There was a problem hiding this comment.
Traced this further using the four repro tests you posted — the same clobbering pattern (a follow-up DDL/ALTER statement reusing 'main' after the real DML) turned out to exist in five more places, not just this loop:
apply_row_filterandapply_constraintswere both naming their ALTER statements'main', directly overwriting the incremental merge's real count.create_table_athad it backwards: the schema-onlyCREATE TABLEwas named'main'while the row-writingINSERT ... BY NAMEwas named'merge into target'.safe_relation_replace's backup-tableDROPwas also named'main', clobbering the count thecreate_table_atfix had just corrected.apply_column_masksandalter_column_commentshad the identical bug, caught in a final holistic review rather than by a repro, since they share the exact same call site (apply_config_changeset) asapply_row_filter/apply_constraints.
On the specific question here: rather than leave the multi-statement selection purely positional-and-undocumented, I consolidated the three copies of the "last statement is `main`" logic (this macro plus two inline copies in incremental.sql) into this one macro, and added a comment defining the rule explicitly: multi-statement strategies build preparatory statements first and their real data-writing statement last, so the last statement is authoritative. That's now also written into AGENTS.md so it doesn't regress again.
I stopped short of inventing per-statement descriptive names for the positional case (e.g. delete+insert's two statements) since there's exactly one real multi-statement strategy today and a fuller naming contract felt speculative — happy to revisit if you'd rather have that now.
All four of your repro tests are merged as permanent regression coverage in tests/functional/adapter/response_ownership/.
Resolves databricks#1641 ### Description For incremental models without enforced contracts, skip the three separate fallback `information_schema` queries for non-null, primary-key, and foreign-key metadata during relation-config reconciliation. Enforced contracts retain the existing metadata reads. The consolidated `DESCRIBE TABLE EXTENDED AS JSON` path and metadata reads for tags, masks, and row filters are unchanged. Because V1 and V2 incremental materializations share this reconciliation path, the fix applies to both. ### Testing - `hatch run unit` — 1358 passed, 4 skipped - `hatch run pytest tests/functional/adapter/incremental/test_incremental_metadata_fetch_skips.py --profile databricks_uc_sql_endpoint -v` — 3 passed - `hatch run pytest tests/functional/adapter/constraints/test_constraints.py::TestIncrementalContractOffPreservesConstraints --profile databricks_uc_sql_endpoint -v` — 1 passed - `hatch run pre-commit run --all-files` — passed ### 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. - [ ] [Optional] I have run the `dbt-databricks-pr-ready` project skill for this PR and addressed its merge-readiness feedback
23443c9 to
dfe5967
Compare
Signed-off-by: Chris Cooper <chris.cooper@relativity.com>
Signed-off-by: Chris Cooper <chris.cooper@relativity.com>
- Deduplicate statement names in loops (statement.sql, incremental.sql) so the first statement's cursor rowcount is not clobbered by later ones; non-loop callers (tags, tblproperties, column_tags) get descriptive names - Exclude bool from _get_rows_affected int guard (bool is int subclass in Python) - Functional test: add skip_profile for HMS cluster, restore pytest.skip guard when rowcount is None, assert rows_affected > 0 instead of hard == 2, drop format-string message assertion (unit-test domain), remove tag query that coupled two unrelated features - Unit tests: cover zero rowcount, missing rowcount attribute, and None cursor Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Chris Cooper <chris.cooper@relativity.com>
- Name the last statement in multi-statement strategies 'main' (not the first), so delete+insert reports the INSERT rowcount instead of DELETE - Assert rows_affected == 2 in functional test; drop pytest.skip fallback that would silently pass if the feature regressed (connector is pinned to 4.4.0 so the count is deterministic) - Move CHANGELOG entry out of the released 1.12.4 section into a new 1.12.5 TBD section, and reclassify from Fixes to Features Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Chris Cooper <chris.cooper@relativity.com>
- Name the last statement in multi-statement strategies 'main' (not the first), so delete+insert reports the INSERT rowcount instead of DELETE - Assert rows_affected == 2 in functional test; drop pytest.skip fallback that would silently pass if the feature regressed (connector is pinned to 4.4.0 so the count is deterministic) - Move CHANGELOG entry out of the released 1.12.4 section into a new 1.12.5 TBD section, and reclassify from Fixes to Features Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dfe5967 to
114c3cc
Compare
|
Rebased this branch onto
|
Signed-off-by: Chris Cooper <chris.cooper@relativity.com>
Signed-off-by: Chris Cooper <chris.cooper@relativity.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Chris Cooper <chris.cooper@relativity.com>
…chema-only CREATE Signed-off-by: Chris Cooper <chris.cooper@relativity.com>
… rows_affected Rename the drop-backup statement from 'main' to 'drop_backup' so it no longer clobbers the row count from the real row-writing INSERT statement (which is already named 'main' by create_table_at). This ensures safe_relation_replace scenarios correctly report rows_affected instead of silently losing that information. Also clean up extra blank lines to match the rest of the file's style. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Chris Cooper <chris.cooper@relativity.com>
…om PR databricks#1632 reviewer's repro) Signed-off-by: Chris Cooper <chris.cooper@relativity.com>
…ements Signed-off-by: Chris Cooper <chris.cooper@relativity.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Chris Cooper <chris.cooper@relativity.com>
Reason: reviewer feedback on this PR expanded past the incremental-only scope to also fix create_table_at, safe_relation_replace, apply_constraints, and apply_row_filter. Signed-off-by: Chris Cooper <chris.cooper@relativity.com>
… real DML rows_affected Signed-off-by: Chris Cooper <chris.cooper@relativity.com>
…om PR databricks#1632 reviewer's repro)
|
Pushed a follow-up series addressing your Aug 20 review (rebased on
All commits are separated by concern if it's easier to review incrementally. Let me know if you'd rather I use descriptive per-statement names for the delete+insert two-statement case instead of the positional last-is-`main` rule — flagged my reasoning for not doing that inline on the thread. |
databricks#351 asked for rows_affected on at least table and incremental materializations, which this PR's shared-component fix covers, so it auto-closes. databricks#1607 also asked for other build metadata beyond rows_affected, which is out of scope here, so it's linked as partially resolved instead. Signed-off-by: Chris Cooper <chris.cooper@relativity.com>
databricks#351 asked for rows_affected on at least table and incremental materializations, which this PR's shared-component fix covers, so it auto-closes. databricks#1607 also asked for other build metadata beyond rows_affected, which is out of scope here, so it's linked as partially resolved instead.
ab16e3c to
baef58c
Compare
…-batch-rows-affected-dco Signed-off-by: Chris Cooper <chris.cooper@relativity.com> # Conflicts: # CHANGELOG.md
Summary
rows_affectedfrom cursor rowcount when available, and include the row count in the adapter response message (for example,OK 3)create_table_at's schema-only CREATE, andsafe_relation_replace's backup drop) from silently overwriting the real DML statement'srows_affectedby naming them explicitly instead of'main'main" logic into a singleexecute_multiple_statementshelperrows_affectedreportingTesting
hatch run code-quality(ruff, ruff format, mypy, uv-lock) passeshatch run unitpasses: 1362 passed, 4 skippedcluster-e2e-dev) has not been run against a live workspace from this environmentRelated
adapter_response#1607 (Add rows_affected and other build metadata to run_results adapter_response) — this PR coversrows_affectedonly, not the other build metadata requested there