planner: add opt-in greedy join reorder seed-by-cost switch#67784
planner: add opt-in greedy join reorder seed-by-cost switch#67784Reminiscent wants to merge 1 commit into
Conversation
ti-chi-bot
Bot
added
release-note-none
|
@Reminiscent I've received your pull request and will start the review. I'll conduct a thorough review covering code quality, potential issues, and implementation details. ⏳ This process typically takes 10-30 minutes depending on the complexity of the changes. ℹ️ Learn more details on Pantheon AI. |
ti-chi-bot
Bot
added
sig/planner
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @Reminiscent. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an opt-in cost-based reseeding path to the greedy join reorder: a new session/sysvar gates selecting the cheapest leaf–leaf join within a connected component as the initial seed. Also prevents speculative connectivity checks from mutating leaf plans and adds tests and plumbing for the new behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Session as SessionVars
participant Reorder as GreedyReorder
participant Conflict as ConflictDetector
participant Seeder as SeedSelector
participant JoinTree as JoinTreeBuilder
Session->>Reorder: read TiDBOptGreedyJoinReorderStartByCost
Reorder->>Conflict: collect connectivity / component indices
Conflict-->>Reorder: CheckConnectionResult (Connected, HasJoinCondition)
Reorder->>Seeder: request candidate leaf-leaf pairs (component)
Seeder->>Reorder: best pair by cumulative join cost
Reorder->>JoinTree: initialize/join seed pair (or fallback to legacy seed)
JoinTree-->>Reorder: jrNode with updated leafCnt
Reorder->>Reorder: continue greedy expansion (betterLegacyGreedyCandidate)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
pkg/sessionctx/variable/session.go (1)
1267-1270: Initialize this new session flag explicitly inNewSessionVars.Line 1267 adds a new optimizer flag, but its default currently depends on Go’s zero-value. Please wire it to
vardef.DefTiDBOptGreedyJoinReorderStartByCostin the constructor for consistency with adjacent optimizer vars and future-proofing.♻️ Suggested change
@@ func NewSessionVars(hctx HookContext) *SessionVars { TiDBOptJoinReorderThreshold: vardef.DefTiDBOptJoinReorderThreshold, TiDBOptEnableAdvancedJoinReorder: vardef.DefTiDBOptEnableAdvancedJoinReorder, + TiDBOptGreedyJoinReorderStartByCost: vardef.DefTiDBOptGreedyJoinReorderStartByCost, TiDBOptJoinReorderThroughSel: vardef.DefTiDBOptJoinReorderThroughSel,As per coding guidelines, “Code SHOULD remain maintainable for future readers” and “Follow existing package-local conventions first”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/sessionctx/variable/session.go` around lines 1267 - 1270, The new session flag TiDBOptGreedyJoinReorderStartByCost is currently left at Go's zero value; update NewSessionVars to explicitly initialize this field using vardef.DefTiDBOptGreedyJoinReorderStartByCost so it follows the pattern of adjacent optimizer vars (e.g., set TiDBOptGreedyJoinReorderStartByCost: vardef.DefTiDBOptGreedyJoinReorderStartByCost in the session vars constructor) ensuring consistent defaults and future-proofing.pkg/sessionctx/vardef/tidb_vars.go (1)
671-675: Clarify the previous heuristic wording in the comment.
"smallest base-node cumulative cost"reads a bit ambiguous; consider phrasing it as seeding from the “smallest base relation” to match the planner terminology used in this area.✏️ Suggested wording tweak
- // instead of the smallest base-node cumulative cost. It is kept opt-in so + // instead of seeding from the smallest base relation. It is kept opt-in soAs per coding guidelines, comments SHOULD explain non-obvious intent clearly for maintainers with basic TiDB familiarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/sessionctx/vardef/tidb_vars.go` around lines 671 - 675, The comment for TiDBOptGreedyJoinReorderStartByCost is ambiguous — replace the phrase "smallest base-node cumulative cost" with clearer planner terminology such as "seeding from the smallest base relation (i.e., the base-node with the smallest cumulative cost)" so it reads that the greedy join reorder picks the first join in a connected component by the cheapest join cost instead of seeding from the smallest base relation; update the comment above the TiDBOptGreedyJoinReorderStartByCost constant accordingly to use this phrasing.tests/integrationtest/t/planner/core/join_reorder2.test (1)
100-114: Consider resetting all touched session vars at the end for better isolation.You already restore
tidb_opt_enable_advanced_join_reorderandtidb_opt_greedy_join_reorder_start_by_cost; restoringtidb_enable_outer_join_reordertoo would make this block safer if future cases are appended below.🧹 Optional cleanup tail
set @@tidb_opt_enable_advanced_join_reorder = 1; set @@tidb_opt_greedy_join_reorder_start_by_cost = 0; +set @@tidb_enable_outer_join_reorder = default;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integrationtest/t/planner/core/join_reorder2.test` around lines 100 - 114, Add a final session reset for tidb_enable_outer_join_reorder to restore test isolation: after the existing resets for tidb_opt_enable_advanced_join_reorder and tidb_opt_greedy_join_reorder_start_by_cost, add a statement to set @@tidb_enable_outer_join_reorder back to its default (e.g., 0) so that tidb_enable_outer_join_reorder, tidb_opt_enable_advanced_join_reorder, and tidb_opt_greedy_join_reorder_start_by_cost are all restored at the end of the test block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/planner/core/joinorder/join_order.go`:
- Around line 735-783: Limit the reseed scan to the connected component that
starts at nodes[start] by first computing the set of indices reachable from
start (using the same connectivity logic/detector used elsewhere) and then only
iterating leftIdx/rightIdx over that reachableIndices set instead of
nodes[start:]; update the nested loops that call checkConnectionAndMakeJoin to
use reachableIndices, so bestNode/bestLeftIdx/bestRightIdx only consider pairs
inside the current component, and defer considering cartesian joins
(applyCartesianFactor/cartesianFactor and allowNoEQ handling) until the global
final-stitch phase rather than during this component-local reseed.
- Around line 754-760: seedGreedyJoinComponentByCost already calls
applyCartesianFactor and embeds cartesianFactor into newNode.cumCost, so
betterGreedyCandidate must not multiply the penalty again; update the calls to
betterGreedyCandidate in seedGreedyJoinComponentByCost (and the other occurrence
around the 785-794 block) to pass a neutral factor (1.0) or otherwise indicate
"no extra cartesian penalty" when newNode.cumCost has already been adjusted,
leaving betterGreedyCandidate's logic unchanged; reference the symbols
newNode.cumCost, cartesianFactor, seedGreedyJoinComponentByCost, and
betterGreedyCandidate to locate and fix the double-application.
In `@pkg/planner/core/rule_join_reorder_greedy.go`:
- Around line 171-232: seedConnectedJoinTreeByCost currently scans all nodes in
s.curJoinGroup and can pick a seed from a different disconnected component;
restrict the seed search to only the nodes in the component being built (the
"current frontier") so later components cannot jump ahead. Concretely: before
the double loop in seedConnectedJoinTreeByCost, compute the set/slice of indices
that belong to the connected component rooted at the current frontier
(reuse/derive logic from constructConnectedJoinTree or use
checkConnectionAndMakeJoin connectivity checks) and then iterate
leftIdx/rightIdx only over that component subset of s.curJoinGroup; keep the
rest of the function (bestLeftIdx/bestRightIdx selection and collapsing into
remaining) but ensure you only collapse nodes from that same component and leave
other components untouched.
---
Nitpick comments:
In `@pkg/sessionctx/vardef/tidb_vars.go`:
- Around line 671-675: The comment for TiDBOptGreedyJoinReorderStartByCost is
ambiguous — replace the phrase "smallest base-node cumulative cost" with clearer
planner terminology such as "seeding from the smallest base relation (i.e., the
base-node with the smallest cumulative cost)" so it reads that the greedy join
reorder picks the first join in a connected component by the cheapest join cost
instead of seeding from the smallest base relation; update the comment above the
TiDBOptGreedyJoinReorderStartByCost constant accordingly to use this phrasing.
In `@pkg/sessionctx/variable/session.go`:
- Around line 1267-1270: The new session flag
TiDBOptGreedyJoinReorderStartByCost is currently left at Go's zero value; update
NewSessionVars to explicitly initialize this field using
vardef.DefTiDBOptGreedyJoinReorderStartByCost so it follows the pattern of
adjacent optimizer vars (e.g., set TiDBOptGreedyJoinReorderStartByCost:
vardef.DefTiDBOptGreedyJoinReorderStartByCost in the session vars constructor)
ensuring consistent defaults and future-proofing.
In `@tests/integrationtest/t/planner/core/join_reorder2.test`:
- Around line 100-114: Add a final session reset for
tidb_enable_outer_join_reorder to restore test isolation: after the existing
resets for tidb_opt_enable_advanced_join_reorder and
tidb_opt_greedy_join_reorder_start_by_cost, add a statement to set
@@tidb_enable_outer_join_reorder back to its default (e.g., 0) so that
tidb_enable_outer_join_reorder, tidb_opt_enable_advanced_join_reorder, and
tidb_opt_greedy_join_reorder_start_by_cost are all restored at the end of the
test block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 07c7ec83-15a7-4889-9e42-b1fc536af02b
📒 Files selected for processing (10)
pkg/planner/core/joinorder/join_order.gopkg/planner/core/rule_join_reorder.gopkg/planner/core/rule_join_reorder_greedy.gopkg/sessionctx/vardef/tidb_vars.gopkg/sessionctx/variable/session.gopkg/sessionctx/variable/setvar_affect.gopkg/sessionctx/variable/sysvar.gopkg/sessionctx/variable/varsutil_test.gotests/integrationtest/r/planner/core/join_reorder2.resulttests/integrationtest/t/planner/core/join_reorder2.test
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #67784 +/- ##
================================================
- Coverage 77.6020% 76.9612% -0.6409%
================================================
Files 1981 1968 -13
Lines 548804 557716 +8912
================================================
+ Hits 425883 429225 +3342
- Misses 122111 127757 +5646
+ Partials 810 734 -76
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
ti-chi-bot
Bot
added
size/XXL
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pkg/planner/core/joinorder/join_order.go (1)
746-764:⚠️ Potential issue | 🟠 MajorSkip pure cartesian candidates during component reseeding.
When
allowNoEQis true, this loop can still seed the component with aTryCreateCartesianCheckResult()pair that only shares the component through some third table. That makes the new switch choose an early cartesian seed, which is broader than the intended “best first real join inside this component” behavior.Suggested fix
checkResult, newNode, err := checkConnectionAndMakeJoin(detector, nodes[leftIdx], nodes[rightIdx], vertexHints, allowNoEQ) if err != nil { return nil, false, err } if newNode == nil { continue } + if !checkResult.HasJoinCondition() { + continue + } if checkResult.NoEQEdge() { if !allowNoEQ { continue } newNode.cumCost, err = applyCartesianFactor(newNode.cumCost, cartesianFactor)Based on learnings, cartesian edges should only be considered after the entire join reorder is complete and there are still disjoint join groups that need to be connected, or when a
leadinghint explicitly requires a cartesian join.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/planner/core/joinorder/join_order.go` around lines 746 - 764, The loop currently accepts joins produced by TryCreateCartesianCheckResult() even when allowNoEQ is true, which seeds components with pure-cartesian candidates; modify the loop in join order seeding (around checkConnectionAndMakeJoin, checkResult, newNode, allowNoEQ) to skip candidates that are pure-cartesian by detecting the TryCreateCartesianCheckResult-origin (e.g., via a new predicate like checkResult.IsPureCartesian() or an existing marker on checkResult) and continue early when allowNoEQ is true unless a leading hint explicitly requires a cartesian join or you are in the final reconnection phase where disjoint groups must be connected; keep the existing cartesian cost application (applyCartesianFactor) for cases that proceed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/planner/core/rule_join_reorder_greedy.go`:
- Around line 171-203: In seedConnectedJoinTreeByCost, the loop over
componentIdxs can pick a cartesian pair when cartesianThreshold > 0; change the
loop so that after calling checkConnectionAndMakeJoin(...) you immediately skip
any isCartesian candidate when cartesianThreshold > 0 (i.e., if isCartesian &&
cartesianThreshold > 0 { continue }) so cartesian edges are not used to seed the
legacy greedy search; place this check before recording the opt var and before
cost/stat derivation to avoid counting or logging cartesian pairs prematurely
(refs: seedConnectedJoinTreeByCost, collectConnectedSeedComponentIndices,
checkConnectionAndMakeJoin, betterLegacyGreedyCandidate).
---
Duplicate comments:
In `@pkg/planner/core/joinorder/join_order.go`:
- Around line 746-764: The loop currently accepts joins produced by
TryCreateCartesianCheckResult() even when allowNoEQ is true, which seeds
components with pure-cartesian candidates; modify the loop in join order seeding
(around checkConnectionAndMakeJoin, checkResult, newNode, allowNoEQ) to skip
candidates that are pure-cartesian by detecting the
TryCreateCartesianCheckResult-origin (e.g., via a new predicate like
checkResult.IsPureCartesian() or an existing marker on checkResult) and continue
early when allowNoEQ is true unless a leading hint explicitly requires a
cartesian join or you are in the final reconnection phase where disjoint groups
must be connected; keep the existing cartesian cost application
(applyCartesianFactor) for cases that proceed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7871d12d-a172-4732-b278-6750898c58fd
📒 Files selected for processing (5)
pkg/planner/core/joinorder/conflict_detector.gopkg/planner/core/joinorder/join_order.gopkg/planner/core/rule_join_reorder_greedy.gotests/integrationtest/r/planner/core/join_reorder2.resulttests/integrationtest/t/planner/core/join_reorder2.test
✅ Files skipped from review due to trivial changes (1)
- tests/integrationtest/t/planner/core/join_reorder2.test
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/integrationtest/r/planner/core/join_reorder2.result
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/integrationtest/t/planner/core/join_reorder2.test (1)
193-210: Good cartesian exclusion test, but cleanup may be incomplete.The test at lines 193-207 properly verifies that reseeding won't start from a pure cartesian pair.
However, the cleanup at lines 208-210 resets
tidb_opt_join_reorder_threshold,tidb_opt_cartesian_join_order_threshold, andtidb_opt_greedy_join_reorder_start_by_cost, but leavestidb_enable_outer_join_reorder(set to 1 at line 100) andtidb_opt_enable_advanced_join_reorder(set to 1 at line 206) at non-default values. If subsequent tests in this file or session depend on default values, this could cause unexpected behavior.♻️ Suggested cleanup additions
set @@tidb_opt_join_reorder_threshold = 0; set @@tidb_opt_cartesian_join_order_threshold = 0; set @@tidb_opt_greedy_join_reorder_start_by_cost = 0; +set @@tidb_enable_outer_join_reorder = default; +set @@tidb_opt_enable_advanced_join_reorder = default;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integrationtest/t/planner/core/join_reorder2.test` around lines 193 - 210, The test resets three session variables but leaves tidb_enable_outer_join_reorder and tidb_opt_enable_advanced_join_reorder altered; update the cleanup at the end of this test (after the existing set ... = 0 lines) to also restore tidb_enable_outer_join_reorder and tidb_opt_enable_advanced_join_reorder to their defaults (rollback the change made earlier where tidb_enable_outer_join_reorder was set to 1 and where tidb_opt_enable_advanced_join_reorder was set to 1) so subsequent tests see expected default settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/integrationtest/t/planner/core/join_reorder2.test`:
- Around line 193-210: The test resets three session variables but leaves
tidb_enable_outer_join_reorder and tidb_opt_enable_advanced_join_reorder
altered; update the cleanup at the end of this test (after the existing set ...
= 0 lines) to also restore tidb_enable_outer_join_reorder and
tidb_opt_enable_advanced_join_reorder to their defaults (rollback the change
made earlier where tidb_enable_outer_join_reorder was set to 1 and where
tidb_opt_enable_advanced_join_reorder was set to 1) so subsequent tests see
expected default settings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 588fd666-58f5-4e3f-a552-5567dae555b3
📒 Files selected for processing (4)
pkg/planner/core/joinorder/join_order.gopkg/planner/core/rule_join_reorder_greedy.gotests/integrationtest/r/planner/core/join_reorder2.resulttests/integrationtest/t/planner/core/join_reorder2.test
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/integrationtest/r/planner/core/join_reorder2.result
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/planner/core/join_reorder_side_effect_test.go (1)
88-94: Keep this regression focused on immutability, not exact wrapper placement.Lines 91-92 require both returned plans to differ from the original inputs. That is stronger than the bug contract and makes the test brittle against harmless cast-normalization changes that only need to wrap one side. The regression is already protected by asserting that
projectionLeafitself stays unchanged.Suggested simplification
newLeft, newRight, usedEdges, joinType := solver.checkConnection(intLeaf, projectionLeaf) require.Len(t, usedEdges, 1) require.Equal(t, base.InnerJoin, joinType.JoinType) - require.NotSame(t, intLeaf, newLeft) - require.NotSame(t, projectionLeaf, newRight) + require.NotNil(t, newLeft) + require.NotNil(t, newRight) require.Len(t, projectionLeaf.Exprs, origExprCnt) require.Equal(t, origSchemaLen, projectionLeaf.Schema().Len())As per coding guidelines, code SHOULD remain maintainable for future readers with basic TiDB familiarity, including readers who are not experts in the specific subsystem/feature.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/planner/core/join_reorder_side_effect_test.go` around lines 88 - 94, The test currently asserts both returned plans differ from the inputs (require.NotSame on newLeft and newRight) which is too strict; update the test in join_reorder_side_effect_test.go to only require the input side that must be wrapped (keep require.NotSame for newLeft vs intLeaf if desired) and remove the require.NotSame assertion for projectionLeaf/newRight so the regression focuses on immutability; retain the existing assertions that projectionLeaf.Exprs and projectionLeaf.Schema() remain unchanged after calling solver.checkConnection(intLeaf, projectionLeaf).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/planner/core/operator/logicalop/logical_projection.go`:
- Around line 695-703: InjectExprAvoidingMutation shallow-copies a
LogicalProjection via proj.LogicalProjectionShallowRef() but leaves any cached
StatsInfo() intact, so after proj.AppendExpr(expr) downstream DeriveStats() can
reuse stale stats and skip NDV recomputation; fix by invalidating the cloned
projection's stats cache (clear or nil-out its StatsInfo) immediately after
proj.LogicalProjectionShallowRef() and before proj.AppendExpr(expr) (i.e., in
InjectExprAvoidingMutation, after calling proj.LogicalProjectionShallowRef() and
before proj.SetSchema/AppendExpr) so NDV will be recomputed correctly during
DeriveStats().
---
Nitpick comments:
In `@pkg/planner/core/join_reorder_side_effect_test.go`:
- Around line 88-94: The test currently asserts both returned plans differ from
the inputs (require.NotSame on newLeft and newRight) which is too strict; update
the test in join_reorder_side_effect_test.go to only require the input side that
must be wrapped (keep require.NotSame for newLeft vs intLeaf if desired) and
remove the require.NotSame assertion for projectionLeaf/newRight so the
regression focuses on immutability; retain the existing assertions that
projectionLeaf.Exprs and projectionLeaf.Schema() remain unchanged after calling
solver.checkConnection(intLeaf, projectionLeaf).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b99a1893-1cd0-440d-a47c-30effb7dccae
📒 Files selected for processing (9)
pkg/planner/core/BUILD.bazelpkg/planner/core/join_reorder_side_effect_test.gopkg/planner/core/joinorder/BUILD.bazelpkg/planner/core/joinorder/conflict_detector.gopkg/planner/core/joinorder/join_order_side_effect_test.gopkg/planner/core/operator/logicalop/logical_projection.gopkg/planner/core/rule_join_reorder.gotests/integrationtest/r/planner/core/join_reorder2.resulttests/integrationtest/t/planner/core/join_reorder2.test
✅ Files skipped from review due to trivial changes (2)
- pkg/planner/core/BUILD.bazel
- tests/integrationtest/t/planner/core/join_reorder2.test
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/planner/core/joinorder/conflict_detector.go
Reminiscent
changed the title
|
@Reminiscent: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1 participant
What problem does this PR solve?
Issue Number: close #67774
Problem Summary:
Greedy join reorder currently seeds each connected join component from the smallest base relation.
That heuristic is not always good enough for outer-join groups. In a query like:
when t2 < t3 << t1, the greedy path may start from t2, which effectively picks t1 left join t2 as the first join even though the outer join cannot reduce below t1's row
count.
The better first choice is to join t3 with t1 first and then left-join t2.
What changed and how does it work?
This PR adds an opt-in system variable:
When this variable is ON, greedy join reorder no longer always starts from the smallest base node. Instead, it enumerates valid first join pairs inside each connected
component and uses the pair with the smallest cumulative join cost as the seed.
The change is applied to both:
A new integration regression test is added to show:
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.
Summary by CodeRabbit
New Features
Behavior Changes
Bug Fixes
Tests