Skip to content

fix(query): bound automatic broadcast join risk - #20383

Draft
dantengsky wants to merge 10 commits into
databendlabs:mainfrom
dantengsky:fix/bound-broadcast-risk-main-minimal
Draft

fix(query): bound automatic broadcast join risk#20383
dantengsky wants to merge 10 commits into
databendlabs:mainfrom
dantengsky:fix/bound-broadcast-risk-main-minimal

Conversation

@dantengsky

@dantengsky dantengsky commented Aug 26, 2026

Copy link
Copy Markdown
Member

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_cardinality largest-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. Explicit enforce_broadcast_join=1 continues to override the automatic guard.

max_cardinality is 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_rows setting defaults to 0, 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

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Validated on the current main base:

  • 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 --nocapture
  • cargo test -p databend-common-settings --test it setting::test_set_settings -- --exact --nocapture
  • cargo clippy -p databend-common-sql -p databend-common-settings --all-targets -- -D warnings
  • cargo fmt --all -- --check
  • git diff --check origin/main...HEAD

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

AI assistance

  • AI usage: A coding agent helped narrow the earlier implementation to the original broadcast-risk scope, adapt it to current main, and draft tests and the PR summary
  • Responsible human: @dantengsky
  • The responsible human has read every line of this diff and can explain each change

This change is Reviewable

@github-actions github-actions Bot added the pr-bugfix this PR patches a bug in codebase label Aug 26, 2026
@dantengsky

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/query/sql/src/planner/plans/aggregate.rs Outdated
Comment thread src/query/sql/src/planner/plans/union_all.rs
@dantengsky

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/query/sql/src/planner/plans/filter.rs Outdated
@dantengsky

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/query/sql/src/planner/plans/scan.rs Outdated
Comment thread src/query/sql/src/planner/plans/join.rs
@dantengsky

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 85ffc78e6c

ℹ️ 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".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix this PR patches a bug in codebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant