planner: handle the projection between the join group (#65367)#68002
planner: handle the projection between the join group (#65367)#68002ti-chi-bot wants to merge 1 commit intopingcap:release-8.5from
Conversation
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
ti-chi-bot
added
do-not-merge/hold
|
[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 |
📝 WalkthroughWalkthroughThis PR enables join reordering through safe Projection operators by introducing hash-cache invalidation for mutated expressions, implementing projection-inlining safety validation, adding join-order utility functions for LEADING hints and derived-column substitution, updating DP and greedy join solvers to handle computed join keys, and introducing session variables to control this behavior. Changes
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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions 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 |
|
@ti-chi-bot: 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. |
|
@ti-chi-bot: The following tests 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. |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/planner/core/rule_join_reorder.go (1)
27-1003:⚠️ Potential issue | 🔴 CriticalUnresolved cherry-pick merge conflicts — file will not compile.
This file contains unresolved Git conflict markers (
<<<<<<< HEAD,=======,>>>>>>> a64f198a6ee) at lines 27, 33, 37, 47, 49, 104, 161, 191, 208, 229, 255, 262, 302–303, 327, 453, 473, 477, 497, 528, 596, 632, 635, 675, 741, 743, 767, 808, 840, 844, 995, 999, and 1003. These markers must be resolved before the file can compile.When resolving, address these integration issues:
Imports: The conflict at lines 27–37 splits the imports. The HEAD side has
planner/util,optimizetrace,plancodec, andtracing; the incoming side addsplannererrorsandintest. Resolution should union both, not pick one. Ensurepkg/planner/core/joinorderis imported (used by incomingSubstituteColsInEqEdges,GetEqEdgeArgsAndCols,AlignJoinEdgeArgs,OuterJoinSideFiltersTouchMultipleLeaves,IsDerivedTableInLeadingHint).Join-type enums: The incoming code uses
base.InnerJoin,base.LeftOuterJoin,base.RightOuterJoin, but these enums live inlogicalopin this branch (seelogical_join.golines 51, 53, 55). All join-type references must uselogicalop.*consistently, notbase.*.Function signatures:
optimizeRecursive(line 368) takes(ctx, p, tracer), but the fallback at line 396 calls it with only 2 args.generateLeadingJoinGroup(line 426 call) uses the old 4-arg signature(curJoinGroup, hintInfo, hasOuterJoin, tracer.opt)returning(bool, []base.LogicalPlan), but the incoming definition (lines 650+) returns(bool, []base.LogicalPlan, error). Pick one consistent signature and update all call sites.makeJoincall at line 737 passesopt, but the incoming signature (line 837) expectsinputOtherConds []expression.Expressioninstead.Undefined in context:
nullExtendedColsat line 324,hasOtherJoinConditionat line 759, andrightShouldPreserveat line 256 are defined only on the incoming side. Verify these belong in release-8.5 semantics; if not, they must be removed during conflict resolution.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/planner/core/rule_join_reorder.go` around lines 27 - 1003, The file has unresolved merge markers; remove all git conflict markers and merge the two sides into a single coherent implementation: (1) unify imports by taking the union (include planner/util, optimizetrace, plancodec, tracing, plannererrors, intest and ensure joinorder is imported), (2) normalize all join type constants to use logicalop.* (e.g., logicalop.InnerJoin/LeftOuterJoin/RightOuterJoin) everywhere, (3) reconcile and standardize the changed function signatures and their call sites — make optimizeRecursive calls match its final signature used in this file, make generateLeadingJoinGroup return (bool, []base.LogicalPlan, error) and update every caller (including the place that previously expected two returns) accordingly, and make makeJoin accept the final parameter list (use inputOtherConds []expression.Expression) and update its callers, (4) either restore or remove usages of undefined symbols (nullExtendedCols, hasOtherJoinCondition, rightShouldPreserve) consistently with the intended incoming changes — define them if needed or remove related logic, and (5) ensure helper functions referenced from joinorder (SubstituteColsInEqEdges, GetEqEdgeArgsAndCols, AlignJoinEdgeArgs, OuterJoinSideFiltersTouchMultipleLeaves, IsDerivedTableInLeadingHint) are called with correct names and that colExprMap merging uses mergeMap; after these edits, run `go build` to fix remaining compilation errors.
🤖 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/BUILD.bazel`:
- Line 297: A new dependency "//pkg/planner/core/joinorder" was added but the
package lacks a BUILD.bazel, causing build failures; add a BUILD.bazel for
pkg/planner/core/joinorder (so Bazel knows the target) by generating it via the
repo tooling — run make bazel_prepare to create
pkg/planner/core/joinorder/BUILD.bazel (ensure it includes the appropriate Go
library target that covers util.go and any other sources in that directory).
In `@pkg/planner/core/rule_join_reorder_dp_test.go`:
- Around line 243-320: The tests call coretestsdk.MockContext() (in
TestInjectedJoinExprMaterializationSafety and the following
TestJoinReorderInlineSafetyGates) but that package isn't imported and a local
MockContext() helper exists in mock.go; replace both calls to
coretestsdk.MockContext() with the local MockContext() function so the tests use
the in-package helper (no new import needed) and ensure any deferred domain
cleanup and ctx usage remain unchanged.
In `@pkg/planner/core/rule_join_reorder_greedy.go`:
- Around line 154-159: Remove the unresolved git conflict markers and fix the
return to match the function's two-value signature (base.LogicalPlan,
[]expression.Expression): call s.makeJoin with the updated signature by passing
s.otherConds (i.e., s.makeJoin(leftPlan, rightPlan, usedEdges, joinType,
s.otherConds)) and return its two results; delete any mention of the undefined
isCartesian and ensure only the two values returned by makeJoin are returned
from this function.
---
Outside diff comments:
In `@pkg/planner/core/rule_join_reorder.go`:
- Around line 27-1003: The file has unresolved merge markers; remove all git
conflict markers and merge the two sides into a single coherent implementation:
(1) unify imports by taking the union (include planner/util, optimizetrace,
plancodec, tracing, plannererrors, intest and ensure joinorder is imported), (2)
normalize all join type constants to use logicalop.* (e.g.,
logicalop.InnerJoin/LeftOuterJoin/RightOuterJoin) everywhere, (3) reconcile and
standardize the changed function signatures and their call sites — make
optimizeRecursive calls match its final signature used in this file, make
generateLeadingJoinGroup return (bool, []base.LogicalPlan, error) and update
every caller (including the place that previously expected two returns)
accordingly, and make makeJoin accept the final parameter list (use
inputOtherConds []expression.Expression) and update its callers, (4) either
restore or remove usages of undefined symbols (nullExtendedCols,
hasOtherJoinCondition, rightShouldPreserve) consistently with the intended
incoming changes — define them if needed or remove related logic, and (5) ensure
helper functions referenced from joinorder (SubstituteColsInEqEdges,
GetEqEdgeArgsAndCols, AlignJoinEdgeArgs,
OuterJoinSideFiltersTouchMultipleLeaves, IsDerivedTableInLeadingHint) are called
with correct names and that colExprMap merging uses mergeMap; after these edits,
run `go build` to fix remaining compilation errors.
🪄 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: a07f1812-8a66-418d-8178-1af9e5c7138d
📒 Files selected for processing (16)
pkg/expression/scalar_function.gopkg/expression/scalar_function_test.gopkg/expression/util.gopkg/planner/core/BUILD.bazelpkg/planner/core/joinorder/util.gopkg/planner/core/operator/logicalop/logical_cte.gopkg/planner/core/rule_join_reorder.gopkg/planner/core/rule_join_reorder_dp.gopkg/planner/core/rule_join_reorder_dp_test.gopkg/planner/core/rule_join_reorder_greedy.gopkg/planner/core/rule_join_reorder_projection_inline.gopkg/sessionctx/vardef/tidb_vars.gopkg/sessionctx/variable/session.gopkg/sessionctx/variable/sysvar.gotests/integrationtest/r/planner/core/join_reorder_through_projection.resulttests/integrationtest/t/planner/core/join_reorder_through_projection.test
| "//pkg/parser/terror", | ||
| "//pkg/planner", | ||
| "//pkg/planner/core/base", | ||
| "//pkg/planner/core/joinorder", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only check: show Bazel-related changes that should be covered by bazel_prepare.
git diff --name-only origin/release-8.5...HEAD -- '*BUILD.bazel' '*.bazel' '*.bzl' 'go.mod' 'go.sum'Repository: pingcap/tidb
Length of output: 105
🏁 Script executed:
#!/bin/bash
# Get current branch info and show changed Bazel-related files
git branch -a | head -20
git status --short | grep -E '(BUILD\.bazel|\.bazel|\.bzl|go\.mod|go\.sum)'Repository: pingcap/tidb
Length of output: 159
🏁 Script executed:
#!/bin/bash
# Show the actual changes to pkg/planner/core/BUILD.bazel
git diff HEAD -- pkg/planner/core/BUILD.bazel | head -100Repository: pingcap/tidb
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Check if DEPS.bzl or other generated metadata was changed
git status --short | grep -E 'DEPS|bazel'
git diff --stat | grep -E 'DEPS|bazel|\.bzl'Repository: pingcap/tidb
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Show files in HEAD commit
git show --name-only HEAD | head -50Repository: pingcap/tidb
Length of output: 1145
🏁 Script executed:
#!/bin/bash
# Show the actual change around line 297 in BUILD.bazel
git show HEAD:pkg/planner/core/BUILD.bazel | sed -n '290,310p'Repository: pingcap/tidb
Length of output: 775
🏁 Script executed:
#!/bin/bash
# Check if DEPS.bzl exists and if it's in the commit
git show --name-only HEAD | grep -i depsRepository: pingcap/tidb
Length of output: 64
🏁 Script executed:
#!/bin/bash
# Check if DEPS.bzl was actually modified or just carried through
git diff HEAD~1 HEAD -- DEPS.bzl | head -50Repository: pingcap/tidb
Length of output: 84
🏁 Script executed:
#!/bin/bash
# Check git log to find PR `#65367`
git log --oneline | grep -i '65367\|joinorder' | head -10Repository: pingcap/tidb
Length of output: 107
🏁 Script executed:
#!/bin/bash
# Show the full diff for BUILD.bazel to see context of the joinorder dependency addition
git diff HEAD~1 HEAD -- pkg/planner/core/BUILD.bazelRepository: pingcap/tidb
Length of output: 86
🏁 Script executed:
#!/bin/bash
# Show all Go files in the commit
git show --name-only HEAD | grep '\.go$' | head -30Repository: pingcap/tidb
Length of output: 844
🏁 Script executed:
#!/bin/bash
# Check what's in pkg/planner/core/joinorder directory
git show HEAD:pkg/planner/core/BUILD.bazel | grep -A 20 "name = \"joinorder\""Repository: pingcap/tidb
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Check if there are new test files
git show --name-only HEAD | grep '_test\.go$' | head -20Repository: pingcap/tidb
Length of output: 741
🏁 Script executed:
#!/bin/bash
# Show all pkg/planner files in the commit
git show --name-only HEAD | grep 'pkg/planner' | head -50Repository: pingcap/tidb
Length of output: 2021
🏁 Script executed:
#!/bin/bash
# Check the actual joinorder target in BUILD.bazel
git show HEAD:pkg/planner/core/BUILD.bazel | grep -B 5 -A 10 'joinorder'Repository: pingcap/tidb
Length of output: 619
🏁 Script executed:
#!/bin/bash
# Check if joinorder directory/package exists
git show HEAD:pkg/planner/core/joinorder/BUILD.bazel | head -30Repository: pingcap/tidb
Length of output: 133
🏁 Script executed:
#!/bin/bash
# Check if joinorder directory exists and what's in it
git show HEAD:pkg/planner/core/joinorder/ 2>&1 | head -50Repository: pingcap/tidb
Length of output: 102
🏁 Script executed:
#!/bin/bash
# List all files under pkg/planner/core in the commit
git show --name-only HEAD | grep 'pkg/planner/core/' | sortRepository: pingcap/tidb
Length of output: 19095
🏁 Script executed:
#!/bin/bash
# Check if joinorder is mentioned anywhere else in Bazel files
git show HEAD:pkg/planner/core/BUILD.bazel | grep -n 'joinorder'Repository: pingcap/tidb
Length of output: 99
Create pkg/planner/core/joinorder/BUILD.bazel for the new joinorder package.
The dependency //pkg/planner/core/joinorder was added to line 297 of pkg/planner/core/BUILD.bazel, but pkg/planner/core/joinorder/BUILD.bazel is missing. The util.go file exists in that directory, but without a BUILD.bazel file defining the Bazel target, the build will fail. Run make bazel_prepare to generate the missing BUILD.bazel file for the joinorder package.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/planner/core/BUILD.bazel` at line 297, A new dependency
"//pkg/planner/core/joinorder" was added but the package lacks a BUILD.bazel,
causing build failures; add a BUILD.bazel for pkg/planner/core/joinorder (so
Bazel knows the target) by generating it via the repo tooling — run make
bazel_prepare to create pkg/planner/core/joinorder/BUILD.bazel (ensure it
includes the appropriate Go library target that covers util.go and any other
sources in that directory).
| func TestInjectedJoinExprMaterializationSafety(t *testing.T) { | ||
| ctx := coretestsdk.MockContext() | ||
| defer func() { | ||
| domain.GetDomain(ctx).StatsHandle().Close() | ||
| }() | ||
| ctx.GetSessionVars().PlanID.Store(-1) | ||
|
|
||
| makeRandPlusCol := func(col *expression.Column) expression.Expression { | ||
| randExpr := expression.NewFunctionInternal(ctx, ast.Rand, types.NewFieldType(mysql.TypeDouble)) | ||
| return expression.NewFunctionInternal(ctx, ast.Plus, types.NewFieldType(mysql.TypeDouble), randExpr, col) | ||
| } | ||
|
|
||
| t.Run("injectExpr keeps repeated rand expressions independent", func(t *testing.T) { | ||
| solver := &baseSingleGroupJoinOrderSolver{ | ||
| ctx: ctx, | ||
| basicJoinGroupInfo: &basicJoinGroupInfo{}, | ||
| } | ||
| plan := newDataSource(ctx, "t", 1) | ||
| baseCol := plan.Schema().Columns[0] | ||
|
|
||
| expr1 := makeRandPlusCol(baseCol) | ||
| expr2 := makeRandPlusCol(baseCol) | ||
| require.True(t, expression.ExpressionsSemanticEqual(expr1, expr2)) | ||
|
|
||
| plan, injected1 := solver.injectExpr(plan, expr1) | ||
| plan, injected2 := solver.injectExpr(plan, expr2) | ||
|
|
||
| proj, ok := plan.(*logicalop.LogicalProjection) | ||
| require.True(t, ok) | ||
| require.Len(t, proj.Exprs, 3) | ||
| require.NotEqual(t, injected1.UniqueID, injected2.UniqueID) | ||
| }) | ||
|
|
||
| t.Run("injectExpr keeps appended expression in child space for existing projections", func(t *testing.T) { | ||
| solver := &baseSingleGroupJoinOrderSolver{ | ||
| ctx: ctx, | ||
| basicJoinGroupInfo: &basicJoinGroupInfo{}, | ||
| } | ||
| plan := newDataSource(ctx, "p", 1) | ||
| baseCol := plan.Schema().Columns[0] | ||
|
|
||
| proj := logicalop.LogicalProjection{Exprs: expression.Column2Exprs(plan.Schema().Columns)}.Init(ctx, 0) | ||
| proj.SetSchema(plan.Schema().Clone()) | ||
| proj.SetChildren(plan) | ||
|
|
||
| derivedExpr := expression.NewFunctionInternal(ctx, ast.Plus, types.NewFieldType(mysql.TypeLonglong), baseCol, expression.NewOne()) | ||
| derivedCol := proj.AppendExpr(derivedExpr) | ||
|
|
||
| exprUsingProjOutput := expression.NewFunctionInternal(ctx, ast.Plus, types.NewFieldType(mysql.TypeLonglong), derivedCol, expression.NewOne()) | ||
| newPlan, injectedCol := solver.injectExpr(proj, exprUsingProjOutput) | ||
|
|
||
| newProj, ok := newPlan.(*logicalop.LogicalProjection) | ||
| require.True(t, ok) | ||
| require.NotNil(t, injectedCol) | ||
| require.Len(t, newProj.Exprs, 3) | ||
|
|
||
| appendedExpr := newProj.Exprs[2] | ||
| expectedExpr := expression.NewFunctionInternal( | ||
| ctx, | ||
| ast.Plus, | ||
| types.NewFieldType(mysql.TypeLonglong), | ||
| derivedExpr, | ||
| expression.NewOne(), | ||
| ) | ||
| require.True(t, expression.ExpressionsSemanticEqual(appendedExpr, expectedExpr)) | ||
|
|
||
| appendedCols := expression.ExtractColumns(appendedExpr) | ||
| require.Len(t, appendedCols, 1) | ||
| require.Equal(t, baseCol.UniqueID, appendedCols[0].UniqueID) | ||
| }) | ||
| } | ||
|
|
||
| func TestJoinReorderInlineSafetyGates(t *testing.T) { | ||
| ctx := coretestsdk.MockContext() | ||
| defer func() { | ||
| domain.GetDomain(ctx).StatsHandle().Close() | ||
| }() | ||
| ctx.GetSessionVars().PlanID.Store(-1) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether coretestsdk is imported and where MockContext helpers are defined/used.
rg -n -C2 'coretestsdk\.MockContext|package coretestsdk|func MockContext' --type=goRepository: pingcap/tidb
Length of output: 1440
🏁 Script executed:
#!/bin/bash
# Check imports and convention in the test file
head -50 pkg/planner/core/rule_join_reorder_dp_test.goRepository: pingcap/tidb
Length of output: 1825
🏁 Script executed:
#!/bin/bash
# Check how other tests in the same file initialize context
rg -n 'MockContext\(\)' pkg/planner/core/rule_join_reorder_dp_test.go -B2 -A2 | head -100Repository: pingcap/tidb
Length of output: 683
Replace coretestsdk.MockContext() with the local MockContext() helper.
coretestsdk is not imported in the file. All other tests in this file use the local MockContext() helper directly (lines 169, 218), which is defined in mock.go within the same package.
Fix
Lines 244 and 316:
- ctx := coretestsdk.MockContext()
+ ctx := MockContext()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/planner/core/rule_join_reorder_dp_test.go` around lines 243 - 320, The
tests call coretestsdk.MockContext() (in
TestInjectedJoinExprMaterializationSafety and the following
TestJoinReorderInlineSafetyGates) but that package isn't imported and a local
MockContext() helper exists in mock.go; replace both calls to
coretestsdk.MockContext() with the local MockContext() function so the tests use
the in-package helper (no new import needed) and ensure any deferred domain
cleanup and ctx usage remain unchanged.
| <<<<<<< HEAD | ||
| return s.makeJoin(leftPlan, rightPlan, usedEdges, joinType, opt) | ||
| ======= | ||
| join, remainOtherConds := s.makeJoin(leftPlan, rightPlan, usedEdges, joinType, s.otherConds) | ||
| return join, remainOtherConds, isCartesian | ||
| >>>>>>> a64f198a6ee (planner: handle the projection between the join group (#65367)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Expected: no conflict-marker output and no use of undefined isCartesian in this file.
rg -n '^(<<<<<<<|=======|>>>>>>>)|\bisCartesian\b' pkg/planner/core/rule_join_reorder_greedy.goRepository: pingcap/tidb
Length of output: 221
🏁 Script executed:
sed -n '140,170p' pkg/planner/core/rule_join_reorder_greedy.go | cat -nRepository: pingcap/tidb
Length of output: 1025
🏁 Script executed:
# Find the function signature and return type
sed -n '145,162p' pkg/planner/core/rule_join_reorder_greedy.go | cat -nRepository: pingcap/tidb
Length of output: 824
🏁 Script executed:
# Check the makeJoin method signature
rg -A 2 'func \(s \*joinReorderSolver\) makeJoin' pkg/planner/core/rule_join_reorder_greedy.goRepository: pingcap/tidb
Length of output: 38
🏁 Script executed:
rg -n 'func.*makeJoin' pkg/planner/core/ --type goRepository: pingcap/tidb
Length of output: 623
🏁 Script executed:
# Also check if this function exists in the joinorder package
rg -n 'func.*makeJoin' pkg/planner/core/joinorder/ --type goRepository: pingcap/tidb
Length of output: 38
🏁 Script executed:
sed -n '996,1010p' pkg/planner/core/rule_join_reorder.go | cat -nRepository: pingcap/tidb
Length of output: 1147
🏁 Script executed:
# Check if this is wrapped in conflict markers or if there's a reason for multiple definitions
sed -n '990,1015p' pkg/planner/core/rule_join_reorder.go | cat -nRepository: pingcap/tidb
Length of output: 1536
Resolve unresolved git conflict and fix function return arity.
This hunk contains unresolved conflict markers and attempts to return three values from a function that declares a two-value return type: (base.LogicalPlan, []expression.Expression). The merge branch also references undefined isCartesian. The correct resolution aligns with the updated makeJoin signature (which changed from accepting opt to inputOtherConds): pass s.otherConds and return only the two values.
Conflict resolution
if len(usedEdges) == 0 {
return nil, nil
}
-<<<<<<< HEAD
- return s.makeJoin(leftPlan, rightPlan, usedEdges, joinType, opt)
-=======
- join, remainOtherConds := s.makeJoin(leftPlan, rightPlan, usedEdges, joinType, s.otherConds)
- return join, remainOtherConds, isCartesian
->>>>>>> a64f198a6ee (planner: handle the projection between the join group (`#65367`))
+ return s.makeJoin(leftPlan, rightPlan, usedEdges, joinType, s.otherConds)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/planner/core/rule_join_reorder_greedy.go` around lines 154 - 159, Remove
the unresolved git conflict markers and fix the return to match the function's
two-value signature (base.LogicalPlan, []expression.Expression): call s.makeJoin
with the updated signature by passing s.otherConds (i.e., s.makeJoin(leftPlan,
rightPlan, usedEdges, joinType, s.otherConds)) and return its two results;
delete any mention of the undefined isCartesian and ensure only the two values
returned by makeJoin are returned from this function.
2 participants
This is an automated cherry-pick of #65367
What problem does this PR solve?
Issue Number: close #50229
Problem Summary:
handle the projection between the join group
What changed and how does it work?
https://docs.google.com/document/d/1x5QDc0i9ks9Jy-I2f_3_lsz3oqu1lFbcb-1h3zAD6a4
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
tidb_opt_enable_advanced_join_reorderandtidb_opt_join_reorder_through_projto control enhanced join reorder optimizationBug Fixes