fix(query): bound automatic broadcast join risk - #20383
Conversation
…isk-main-minimal # Conflicts: # src/query/sql/src/planner/plans/join.rs
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ae09bb6f9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09d1e679e2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2f7fd47ac
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
Automatic broadcast join selection currently relies on expected cardinality. When a derived build subtree is severely underestimated, the optimizer can replicate a much larger input across the cluster.
This change keeps a separate
max_cardinalitylargest-source heuristic for broadcast-risk decisions and propagates it through relational operators. Both Cascades and non-Cascades normal join distribution selection inspect the actual build child/group statistics. Automatic broadcast is rejected when a finite source risk is more than 1000x the expected build cardinality. Unknown source sizes retain the existing distribution choice instead of changing unrelated plans. Explicitenforce_broadcast_join=1continues to override the automatic guard.max_cardinalityis intentionally not a proof-grade upper bound on output rows: this PR does not model many-to-many join fan-out or arbitrary SRF expansion. It addresses the observed failure mode where a build derived from a very large source is estimated as tiny; complete output-multiplicity protection belongs in a separate join-type-aware design.The optional
max_broadcast_join_build_rowssetting defaults to0, so the absolute largest-source threshold is disabled unless configured. The severe source-underestimation guard remains enabled. This keeps the default change scoped to the production failure mode while allowing deployments to add a workload-specific source-size threshold.Scope is intentionally limited to the original broadcast-risk fix. It does not add proof-grade NDV tracking or special handling for spatial and nullable mark joins.
A synthetic optimizer replay covers a derived side with a small expected cardinality but a large source risk: the parent join uses hash exchange instead of broadcasting that side, while its genuinely small inner build remains broadcast.
Tests
Validated on the current
mainbase:cargo test -p databend-common-sql --lib(119 passed on the current main base)TEST_SUBDIR=regressions cargo test -p databend-common-sql --test it -- planner::test_lite_replay_service_optimizer_cases --exact --nocapturecargo test -p databend-common-settings --test it setting::test_set_settings -- --exact --nocapturecargo clippy -p databend-common-sql -p databend-common-settings --all-targets -- -D warningscargo fmt --all -- --checkgit diff --check origin/main...HEADType of change
AI assistance
This change is