feat: add skip_optimize model config to opt out of post-materialization OPTIMIZE - #1485
Conversation
e720276 to
55a5713
Compare
…tion OPTIMIZE Adds a `skip_optimize` model config that lets users opt out of the post-materialization OPTIMIZE call without removing zorder/liquid_clustered_by/auto_liquid_cluster from the table definition. Complements the existing run-wide DATABRICKS_SKIP_OPTIMIZE var by allowing project-, folder-, or model-level opt-out via standard dbt config inheritance. Resolves databricks#703 Co-authored-by: Isaac
55a5713 to
1f3e235
Compare
sd-db
left a comment
There was a problem hiding this comment.
Overall change looks good, added a minor comment on hardening.
We would also need to make a docs PR for the new config. Also worth to note in the same on how this config interacts with DATABRICKS_SKIP_OPTIMIZE (DATABRICKS_SKIP_OPTIMIZE cannot be overridden per-model with skip_optimize: false)
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
…hiness Addresses review feedback from sd-db: using `| as_bool` makes the check robust to string values like "true"/"false" that Jinja2 would treat as truthy (non-empty strings). Also registers the as_bool filter in the macro test harness so tests remain runnable without a full dbt env. Co-authored-by: Isaac
|
/integration-test |
|
Integration tests dispatched for PR #1485 by @sd-db. Track progress in the Actions tab. |
|
Integration results for PR #1485 — UC cluster ✅ success · SQL warehouse ✅ success · All-purpose cluster ✅ success · Shard coverage ✅ success |
|
Thanks for the review! Opened the docs PR for the new config: dbt-labs/docs.getdbt.com#9496 It documents |
…ring tests (#1573) ## Description Adds functional-test coverage for the `skip_optimize` model config (#1485) and strengthens two existing liquid-clustering tests to assert server-observable state instead of log substrings. Test-only — no runtime change. ### `skip_optimize` functional coverage (#1485) `skip_optimize=true` short-circuits `databricks__optimize` to a no-op while the clustering declaration stays on the table. This was previously only unit-tested (`tests/unit/macros/relations/test_optimize_macros.py`). New functional tests assert the end-to-end effect on a live warehouse: - **`TestTableSkipOptimizeEffect`** — on a `table` model, `liquid_clustered_by` columns are present AND no `OPTIMIZE` appears in `DESCRIBE HISTORY`. - **`TestIncrementalSkipOptimizeEffect`** — same on an `incremental` model (the high-churn use case from #703), exercising the distinct incremental `optimize()` call site. The `skip_optimize=false`/unset control (OPTIMIZE still runs) is already covered by the existing `TestTableLiquidClusteringEffect`. ### Test strengthening (log substrings → server state) - **`TestLiquidClustering`**: replaced `assert "optimize" in logs` with a `DESCRIBE HISTORY` check that an `OPTIMIZE` operation actually ran. - **`TestAutoLiquidClustering`**: it previously used the non-auto fixture, making it a silent duplicate of `TestLiquidClustering` that never exercised auto clustering. Repointed to the auto fixture; it now asserts `clusterByAuto=true` via `SHOW TBLPROPERTIES`, adding auto-clustering-on-incremental coverage. ### Testing All pass on `databricks_uc_sql_endpoint`: - `tests/functional/adapter/liquid_clustering/` (both files) + `tests/functional/adapter/zorder/` — **11 passed**. - `ruff` / `ruff format` / `mypy` clean on the changed files. ### Checklist - [x] I have run this code in development and it appears to resolve the stated issue - [x] This PR includes tests - [x] No `CHANGELOG.md` entry — test-only change with no runtime/user-facing impact
Resolves #703
Description
Adds a
skip_optimizemodel config that lets users opt out of the post-materializationOPTIMIZEcall without removingzorder/liquid_clustered_by/auto_liquid_clusterfrom the table definition.Motivation: the existing opt-out today is the run-wide
DATABRICKS_SKIP_OPTIMIZEvar, which forces an all-or-nothing decision for the entire invocation. Several users in #703 asked for a config-level opt-out so they can:OPTIMIZEto Predictive Optimization while keepingauto_liquid_cluster=trueon the tableOPTIMIZEonly for specific high-churn models and let it run for the restOPTIMIZEout of band (workflow / job) instead of on the dbt critical pathBehavior:
skip_optimize(bool, defaultfalse)databricks__optimizeshort-circuits to a no-op even ifzorder/liquid_clustered_by/auto_liquid_clusteris set on the model — the clustering declaration remains in the table DDL, only theOPTIMIZESQL emission is suppressedDATABRICKS_SKIP_OPTIMIZEvar is unchanged (still skips run-wide)Docs follow-up
User-facing config reference lives in
dbt-labs/docs.getdbt.com(databricks-configs.md). A companion docs PR will be opened there to documentskip_optimizealongside the existingzorder/liquid_clustered_by/auto_liquid_clusterentries.Checklist
CHANGELOG.mdand added information about my change to the "dbt-databricks next" section.