Repository navigation
Conversation
cf46bfa to
de18aea
Compare
workflow-authentication-public
Bot
added
the
needs-docs
|
Docs drift check: this PR looks like it needs doc updates.
|
There was a problem hiding this comment.
🟡 Changes recommended
Both new changelog entries lack the required PR links, and the first needs shortening to the prescribed one or two sentences.
1 open issue
What changed in this PR
This PR introduces is_distributed=True for standard table and incremental models, allowing dbt-core to recognize distributed models for microbatch scheduling and contract validation.
Changes:
- Routes standard materializations through shared distributed macros while retaining deprecated materialization names with warnings.
- Adds distributed microbatch support and centralizes distributed-model detection.
- Updates documentation and tests for the new configuration and backward compatibility.
| File | Description |
|---|---|
tests/unit/test_is_distributed.py |
Tests flag parsing, legacy names, and invalid values. |
tests/integration/adapter/incremental/test_incremental_microbatch.py |
Adds distributed microbatch coverage. |
tests/integration/adapter/incremental/test_distributed_incremental.py |
Migrates incremental fixtures to the flag. |
tests/integration/adapter/grants/test_distributed_grants.py |
Updates distributed grants configuration and assertions. |
tests/integration/adapter/constraints/fixtures_constraints.py |
Migrates contract fixtures with schema-change settings. |
tests/integration/adapter/clickhouse/test_clickhouse_table_ttl.py |
Uses the flag for distributed TTL coverage. |
tests/integration/adapter/clickhouse/test_clickhouse_table_materializations.py |
Tests legacy warnings and project-level configuration. |
tests/integration/adapter/aliases/test_aliases.py |
Migrates distributed alias fixtures. |
docs/materializations.mdx |
Documents distributed configuration and microbatch support. |
docs/index.mdx |
Updates the distributed feature listing. |
docs/dbt-core-v2-fusion-and-platform.mdx |
Updates distributed configuration terminology. |
dbt/include/clickhouse/macros/materializations/table.sql |
Routes flagged tables to distributed logic. |
dbt/include/clickhouse/macros/materializations/incremental/incremental.sql |
Adds distributed routing and shared microbatch logic. |
dbt/include/clickhouse/macros/materializations/incremental/distributed_incremental.sql |
Extracts distributed logic and adds microbatch handling. |
dbt/include/clickhouse/macros/materializations/distributed_table.sql |
Extracts distributed logic and adds deprecation warnings. |
dbt/include/clickhouse/macros/materializations/clone.sql |
Uses centralized detection for clone eligibility. |
dbt/adapters/clickhouse/impl.py |
Adds distributed detection and adapts model settings. |
CHANGELOG.md |
Records the configuration migration and microbatch support. |
Review effort: Balanced
de18aea to
bc78d16
Compare
bc78d16 to
73a0a79
Compare
canbekley
changed the title
73a0a79 to
0c377a2
Compare
…ributed-from-materialization
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0036c0f. Configure here.
| {% elif incremental_strategy == 'delete_insert' %} | ||
| {% do clickhouse__incremental_delete_insert(existing_relation, unique_key, incremental_predicates, True) %} | ||
| {% elif incremental_strategy == 'microbatch' %} | ||
| {% do clickhouse__incremental_microbatch(existing_relation, unique_key, incremental_predicates, True) %} |
There was a problem hiding this comment.
Deprecated microbatch path skips batching
Medium Severity
The new microbatch branch lives in the shared clickhouse__distributed_incremental macro, which the deprecated materialized='distributed_incremental' wrapper also calls. dbt-core only selects MicrobatchModelRunner when materialized == 'incremental', so that path never sets __dbt_internal_microbatch_event_time_start/end and never event-time-filters refs. The run still takes the microbatch branch and performs a single unwindowed delete+insert over the full compiled SQL, so models that keep the old materialization name can skip batch windows and delete/insert the wrong rows.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0036c0f. Configure here.
There was a problem hiding this comment.
good catch. will fix this by throwing an exception when the deprecated materialized='distributed_incremental' configuration is used together with incremental_strategy='microbatch'
Summary
distributed_tableanddistributed_incrementalaretableandincrementalwith a Distributed table on top, but dbt-core does not treat them as such. dbt-core checksmaterialized == "incremental"in Python, which theis_incremental()macro override cannot reach:get_runner_type()never runs distributed models in microbatch mode.NodeConfig.__post_init__()skips the contracton_schema_changevalidation fordistributed_incremental.This PR introduces a model config
is_distributedas the standard way to configure distributed models:config(materialized='incremental', is_distributed=True). Configuring them withmaterialized='distributed_table'or'distributed_incremental'keeps working, but is deprecated and logs a warning.Building on this, the
microbatchstrategy is now supported for distributed incremental models:config(materialized='incremental', is_distributed=True, incremental_strategy='microbatch', ...)Supersedes #444.
Additional notes
is_distributedaccepts booleans and the strings'true'/'false', and raises an error for other values. Other materializations ignore it, so it can be set for a whole folder with+is_distributed.is_distributed=Truemust seton_schema_changetoappend_new_columnsorfail, as for anyincrementalmodel.--warn-erroruntil models migrate.Out of scope
The distributed implementations are only moved into macros, apart from microbatch support. Merging them into the
tableandincrementalmaterializations, as suggested in #439, may follow.Checklist
docswas updated with further explanations or tutorialsNote
Medium Risk
Touches core materialization macros and adapter settings for distributed workloads; misconfiguration could break cluster builds, though mitigated by warnings and tests.
Overview
Introduces the
is_distributedmodel flag so standardtable/incrementalmaterializations delegate to the ClickHouse distributed macros, while legacydistributed_*configs remain but emit deprecation warnings. Extends distributed incremental flows with microbatch support, shared predicate helpers, and updated clone/contract logic, plus refreshed docs, README mentions, and broad test coverage (unit + cluster integration) for the new behavior.Reviewed by Cursor Bugbot for commit 0036c0f. Bugbot is set up for automated code reviews on this repo. Configure here.