Skip to content

planner: pre-refactor for join reorder conflict detection algorithm #68870

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
guo-shaoge wants to merge 6 commits into
base: release-8.5
Choose a base branch
from
+1,215 −572
Open

planner: pre-refactor for join reorder conflict detection algorithm #68870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c546f85
parser: support recursive parentheses in LEADING hint
guo-shaoge Jun 2, 2026
8f597b5
fix
guo-shaoge Jun 2, 2026
36b2bf6
planner: pre-refactor for join reorder conflict detection algorithm
guo-shaoge Jun 2, 2026
31d66f0
Merge branch 'cp_recursive_join_hint' of github.com:guo-shaoge/tidb i…
guo-shaoge Jun 2, 2026
e0485fa
fix
guo-shaoge Jun 2, 2026
153d1cc
Merge branch 'cp_recursive_join_hint' of github.com:guo-shaoge/tidb i…
guo-shaoge Jun 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions pkg/expression/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,22 @@ func (s *Schema) Clone() *Schema {
return schema
}

// Equal checks if two schemas are equal.
func (s *Schema) Equal(other *Schema) bool {
if s == nil || other == nil {
return s == other
}
if len(s.Columns) != len(other.Columns) {
return false
}
for i, col := range s.Columns {
if !col.EqualColumn(other.Columns[i]) {
return false
}
}
return true
}

// ExprReferenceSchema checks if any column of this expression are from the schema.
func ExprReferenceSchema(expr Expression, schema *Schema) bool {
switch v := expr.(type) {
Expand Down
94 changes: 91 additions & 3 deletions pkg/parser/ast/misc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3896,6 +3896,16 @@ type HintTimeRange struct {
To string
}

// LeadingList represents a nested structure in LEADING hints.
// It could be *HintTable or LeadingList
//
// eg: LEADING(a, (b, c), d)
// will be parsed into a LeadingList like:
// Items = [HintTable("a"), LeadingList{[HintTable("b"), HintTable("c")]}, HintTable("d")]
type LeadingList struct {
Items []interface{}
}

// HintSetVar is the payload of `SET_VAR` hint
type HintSetVar struct {
VarName string
Expand All @@ -3910,6 +3920,25 @@ type HintTable struct {
PartitionList []model.CIStr
}

// FlattenLeadingList collects all HintTable nodes from a possibly nested LeadingList into a flat slice.
// Note:
// - Only table names are preserved.
func FlattenLeadingList(list *LeadingList) []HintTable {
if list == nil {
return nil
}
var result []HintTable
for _, item := range list.Items {
switch t := item.(type) {
case *HintTable:
result = append(result, *t)
case *LeadingList:
result = append(result, FlattenLeadingList(t)...)
}
}
return result
}

func (ht *HintTable) Restore(ctx *format.RestoreCtx) {
if !ctx.Flags.HasWithoutSchemaNameFlag() {
if ht.DBName.L != "" {
Expand All @@ -3935,11 +3964,52 @@ func (ht *HintTable) Restore(ctx *format.RestoreCtx) {
}
}

func (lt *LeadingList) RestoreWithQB(ctx *format.RestoreCtx, qbName model.CIStr, needParen bool, isTop bool, qbOnTable bool) error {
if lt == nil || len(lt.Items) == 0 {
return nil
}
if needParen {
ctx.WritePlain("(")
}

currentQBName := qbName // hint level QBName

for i, item := range lt.Items {
if i > 0 {
ctx.WritePlain(", ")
}

switch t := item.(type) {
case *HintTable:
if i == 0 && currentQBName.L != "" && !qbOnTable {
ctx.WriteKeyWord("@")
ctx.WriteName(currentQBName.String())
ctx.WritePlain(" ")
t.Restore(ctx)
currentQBName = model.CIStr{}
} else {
t.Restore(ctx)
}
case *LeadingList:
if err := t.RestoreWithQB(ctx, currentQBName, true, false, qbOnTable); err != nil {
return err
}
currentQBName = model.CIStr{}
default:
return fmt.Errorf("unexpected type in LeadingList: %T", t)
}
}
if needParen {
ctx.WritePlain(")")
}
return nil
Comment on lines +3967 to +4005
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 2, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve the hint-level QB before a parenthesized first LEADING group.

When the first LEADING element is nested, Line 3994 pushes QBName into the child list, so Restore emits LEADING((@QB a, b), c) instead of LEADING(@QB (a, b), c). That breaks restore/parse round-tripping for valid hints with a hint-level query block.

Suggested fix 
 	case *LeadingList:
-		if err := t.RestoreWithQB(ctx, currentQBName, true, false, qbOnTable); err != nil {
+		childQBName := currentQBName
+		if i == 0 && childQBName.L != "" && !qbOnTable {
+			ctx.WriteKeyWord("@")
+			ctx.WriteName(childQBName.String())
+			ctx.WritePlain(" ")
+			childQBName = model.CIStr{}
+		}
+		if err := t.RestoreWithQB(ctx, childQBName, true, false, qbOnTable); err != nil {
 			return err
 		}
 		currentQBName = model.CIStr{}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/parser/ast/misc.go` around lines 3967 - 4005, The bug is that when the
first LeadingList item is itself a nested LeadingList, RestoreWithQB pushes the
hint-level QB into the child causing LEADING((`@qb` a, b), c) instead of
LEADING(`@qb` (a, b), c); modify LeadingList.RestoreWithQB so that in the case
"case *LeadingList" if i == 0 and currentQBName.L != "" and !qbOnTable you emit
the "@<qb>" (ctx.WriteKeyWord("@"); ctx.WriteName(currentQBName.String());
ctx.WritePlain(" ")) before restoring the child and then call t.RestoreWithQB
with an empty model.CIStr for qbName (so the child does not receive the QB),
afterwards clear currentQBName; otherwise keep the existing behavior of passing
currentQBName into the child.

}

// Restore implements Node interface.
func (n *TableOptimizerHint) Restore(ctx *format.RestoreCtx) error {
ctx.WriteKeyWord(n.HintName.String())
ctx.WritePlain("(")
if n.QBName.L != "" {
if n.HintName.L != "leading" && n.QBName.L != "" {
if n.HintName.L != "qb_name" {
ctx.WriteKeyWord("@")
}
Expand All @@ -3955,7 +4025,7 @@ func (n *TableOptimizerHint) Restore(ctx *format.RestoreCtx) error {
ctx.WritePlain(")")
return nil
}
if n.QBName.L != "" {
if n.HintName.L != "leading" && n.QBName.L != "" {
ctx.WritePlain(" ")
}
// Hints with args except query block.
Expand All @@ -3966,8 +4036,26 @@ func (n *TableOptimizerHint) Restore(ctx *format.RestoreCtx) error {
ctx.WriteName(n.HintData.(string))
case "nth_plan":
ctx.WritePlainf("%d", n.HintData.(int64))
case "leading":
if list, ok := n.HintData.(*LeadingList); ok && list != nil {
// if table level QBName or not
qbOnTable := false
if len(n.Tables) > 0 && n.Tables[0].QBName.L != "" {
qbOnTable = true
}
if err := list.RestoreWithQB(ctx, n.QBName, false, true, qbOnTable); err != nil {
return err
}
} else {
for i, table := range n.Tables {
if i != 0 {
ctx.WritePlain(", ")
}
table.Restore(ctx)
}
}
case "tidb_hj", "tidb_smj", "tidb_inlj", "hash_join", "hash_join_build", "hash_join_probe", "merge_join", "inl_join",
"broadcast_join", "shuffle_join", "inl_hash_join", "inl_merge_join", "leading", "no_hash_join", "no_merge_join",
"broadcast_join", "shuffle_join", "inl_hash_join", "inl_merge_join", "no_hash_join", "no_merge_join",
"no_index_join", "no_index_hash_join", "no_index_merge_join":
for i, table := range n.Tables {
if i != 0 {
Expand Down
10 changes: 10 additions & 0 deletions pkg/parser/ast/misc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,13 +253,23 @@ func TestTableOptimizerHintRestore(t *testing.T) {
{"HASH_JOIN_PROBE(t1)", "HASH_JOIN_PROBE(`t1`)"},
{"LEADING(t1)", "LEADING(`t1`)"},
{"LEADING(t1, c1)", "LEADING(`t1`, `c1`)"},
{"LEADING((t1, c1), t2)", "LEADING((`t1`, `c1`), `t2`)"},
{"LEADING(t1, (c1, t2))", "LEADING(`t1`, (`c1`, `t2`))"},
{"LEADING(((t1, c1), t2), t3)", "LEADING(((`t1`, `c1`), `t2`), `t3`)"},
{"LEADING(t1, (c1, (t2, t3)))", "LEADING(`t1`, (`c1`, (`t2`, `t3`)))"},
{"LEADING(t1, c1, t2)", "LEADING(`t1`, `c1`, `t2`)"},
{"LEADING(@sel1 t1, c1)", "LEADING(@`sel1` `t1`, `c1`)"},
{"LEADING(@sel1 t1)", "LEADING(@`sel1` `t1`)"},
{"LEADING(@sel1 t1, c1, t2)", "LEADING(@`sel1` `t1`, `c1`, `t2`)"},
{"LEADING(@sel1 t1, (c1, t2))", "LEADING(@`sel1` `t1`, (`c1`, `t2`))"},
{"LEADING(@sel1 t1, (c1, t2), d3)", "LEADING(@`sel1` `t1`, (`c1`, `t2`), `d3`)"},
{"LEADING(t1@sel1)", "LEADING(`t1`@`sel1`)"},
{"LEADING(t1@sel1, c1)", "LEADING(`t1`@`sel1`, `c1`)"},
{"LEADING(t1@sel1, c1, t2)", "LEADING(`t1`@`sel1`, `c1`, `t2`)"},
{"LEADING((t1@sel1, c1), t2)", "LEADING((`t1`@`sel1`, `c1`), `t2`)"},
{"LEADING(t1@sel1, (c1, t2))", "LEADING(`t1`@`sel1`, (`c1`, `t2`))"},
{"LEADING(t1@sel1, c1, t2, d3)", "LEADING(`t1`@`sel1`, `c1`, `t2`, `d3`)"},
{"LEADING(t1@sel1, (c1, t2), d3)", "LEADING(`t1`@`sel1`, (`c1`, `t2`), `d3`)"},
{"MAX_EXECUTION_TIME(3000)", "MAX_EXECUTION_TIME(3000)"},
{"MAX_EXECUTION_TIME(@sel1 3000)", "MAX_EXECUTION_TIME(@`sel1` 3000)"},
{"USE_INDEX_MERGE(t1 c1)", "USE_INDEX_MERGE(`t1` `c1`)"},
Expand Down
Loading