Skip to content

Comments

planner: fix plan explore can not detect the specific index path.#65709

Merged
ti-chi-bot[bot] merged 4 commits intopingcap:masterfrom
AilinKid:fix-plan-explore-index-hint
Feb 2, 2026
Merged

planner: fix plan explore can not detect the specific index path.#65709
ti-chi-bot[bot] merged 4 commits intopingcap:masterfrom
AilinKid:fix-plan-explore-index-hint

Conversation

@AilinKid
Copy link
Contributor

What problem does this PR solve?

Issue Number: close #65708

Problem Summary:
Explain Explore can miss alternative index paths for single-table queries when statistics favor one index (e.g., always picking a and never exploring b), which makes it
hard to surface candidate plans for plan binding or verification.

What changed and how does it work?

  • Plan generation now enumerates candidate single-table indexes from WHERE predicate columns and explores them via USE_INDEX hints.
  • The exploration is limited to single-table SELECT with predicates and skips subqueries to avoid unsafe hinting.
  • Added a unit test to verify that EXPLAIN EXPLORE returns both index:a and index:b plans.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No need to test
    • I checked and no code files have been changed.

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Copilot AI review requested due to automatic review settings January 21, 2026 16:45
@ti-chi-bot ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 21, 2026
@tiprow
Copy link

tiprow bot commented Jan 21, 2026

Hi @AilinKid. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Details

Instructions 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.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an issue where EXPLAIN EXPLORE could miss alternative index paths for single-table queries when statistics favor one index over another. The fix allows the plan exploration to enumerate and test all candidate indexes that could be used based on the WHERE predicate columns.

Changes:

  • Added index hint exploration to the breadth-first plan search algorithm
  • Implemented column extraction from WHERE predicates to identify candidate indexes
  • Extended the state representation to include index hints alongside existing leading hints and optimizer variables

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
pkg/bindinfo/binding_plan_generation.go Adds indexHint struct, updates state to track index hints, implements extractSelectIndexHints to identify candidate indexes from WHERE predicates, and integrates index hint exploration into the BFS plan search
pkg/bindinfo/binding_auto_test.go Adds test case TestExplainExploreIndexHints to verify that EXPLAIN EXPLORE returns plans for both index a and index b
pkg/bindinfo/BUILD.bazel Adds dependency on pkg/meta/model and increments test shard count from 48 to 49

@codecov
Copy link

codecov bot commented Jan 21, 2026

Codecov Report

❌ Patch coverage is 81.90476% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.6318%. Comparing base (c2202e1) to head (ca41e03).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #65709        +/-   ##
================================================
- Coverage   77.7795%   77.6318%   -0.1478%     
================================================
  Files          2001       1923        -78     
  Lines        545624     533459     -12165     
================================================
- Hits         424384     414134     -10250     
+ Misses       119578     119319       -259     
+ Partials       1662          6      -1656     
Flag Coverage Δ
integration 41.4752% <0.0000%> (-6.6845%) ⬇️
unit 76.7577% <81.9047%> (+0.3459%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
dumpling 56.7974% <ø> (ø)
parser ∅ <ø> (∅)
br 48.8783% <ø> (-12.1125%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

}
}
for _, indexHint := range possibleIndexHints {
newState := newStateWithIndexHint(currState, indexHint)
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like we only consider one index in a state, if there are multiple tables, should we consider multiple index hints?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

table levels, and for each table, there are multi index choices? make sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i will do it in next pr


func extractSelectIndexHints(sctx sessionctx.Context, node ast.StmtNode, tableNames []*tableName) []*indexHint {
selStmt, isSel := node.(*ast.SelectStmt)
if !isSel || selStmt.Where == nil || len(tableNames) != 1 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why we have such restriction that only support one table?
Does it means explain explore will not support explore index for multiple table query?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we will expand to all tableNames in next pr

Copy link
Collaborator

Choose a reason for hiding this comment

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

Guess we should have a sys var to control this behavior.

tk.MustExec("use test")
tk.MustExec(`create table t (a int, b int, c int, key(a), key(b))`)

rows := tk.MustQuery(`explain explore select * from t where a=1 and b=1`).Rows()
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion: add case that has alias table name to cover this situation

Copy link
Collaborator

@guo-shaoge guo-shaoge Jan 26, 2026

Choose a reason for hiding this comment

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

looks like we failed to explore index b when adding table alias:

TiDB root@127.0.0.1:test> explain explore select 1 from t t_alias where a=1 and b=1 and c like "%xx%"\G
***************************[ 1. row ]***************************
statement                  | select 1 from t t_alias where a=1 and b=1 and c like "%xx%"
binding_hint               | use_index(@`sel_1` `test`.`t_alias` )
plan                       | Projection	1.00	root		1->Column#5
└─TableReader	1.00	root		data:Selection
  └─Selection	1.00	cop[tikv]		eq(test.t.a, 1), eq(test.t.b, 1), like(cast(test.t.c, var_string(20)), "%xx%", 92)
    └─TableFullScan	2000.00	cop[tikv]	table:t_alias	keep order:false, stats:partial[a:allEvicted, b:allEvicted, a:allEvicted...(more: 2 allEvicted)]
plan_digest                | 2d880578187453ef4bec09bd049c955f5e558c549d4e6c0f937b557ec82e8ea8
avg_latency                | 0
exec_times                 | 0
avg_scan_rows              | 0
avg_returned_rows          | 0
latency_per_returned_row   | 0
scan_rows_per_returned_row | 0
recommend                  |
reason                     |
explain_analyze            | EXPLAIN ANALYZE '2d880578187453ef4bec09bd049c955f5e558c549d4e6c0f937b557ec82e8ea8'
binding                    | CREATE GLOBAL BINDING FROM HISTORY USING PLAN DIGEST '2d880578187453ef4bec09bd049c955f5e558c549d4e6c0f937b557ec82e8ea8'
***************************[ 2. row ]***************************
statement                  | select 1 from t t_alias where a=1 and b=1 and c like "%xx%"
binding_hint               | use_index(@`sel_1` `test`.`t_alias` `a`), no_order_index(@`sel_1` `test`.`t_alias` `a`)
plan                       | Projection	1.00	root		1->Column#5
└─IndexLookUp	1.00	root
  ├─IndexRangeScan(Build)	2.00	cop[tikv]	table:t_alias, index:a(a)	range:[1,1], keep order:false, stats:partial[a:allEvicted, b:allEvicted, a:allEvicted...(more: 2 allEvicted)]
  └─Selection(Probe)	1.00	cop[tikv]		eq(test.t.b, 1), like(cast(test.t.c, var_string(20)), "%xx%", 92)
    └─TableRowIDScan	2.00	cop[tikv]	table:t_alias	keep order:false, stats:partial[a:allEvicted, b:allEvicted, a:allEvicted...(more: 2 allEvicted)]
plan_digest                | 53a2f3b7e7fd761ec1e6729d7680306785fae4ccb945f1a6790185ad3c586dd0
avg_latency                | 0
exec_times                 | 0
avg_scan_rows              | 0
avg_returned_rows          | 0
latency_per_returned_row   | 0
scan_rows_per_returned_row | 0
recommend                  |
reason                     |
explain_analyze            | EXPLAIN ANALYZE '53a2f3b7e7fd761ec1e6729d7680306785fae4ccb945f1a6790185ad3c586dd0'
binding                    | CREATE GLOBAL BINDING FROM HISTORY USING PLAN DIGEST '53a2f3b7e7fd761ec1e6729d7680306785fae4ccb945f1a6790185ad3c586dd0'
2 rows in set
Time: 0.482s

Copy link
Contributor Author

Choose a reason for hiding this comment

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

make sense, addressed


func (e *predicateColumnExtractor) Enter(in ast.Node) (node ast.Node, skipChildren bool) {
switch n := in.(type) {
case *ast.SubqueryExpr:
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: why skip SubqueryExpr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

different select block will mix the column name
select a from t, (select a from t2) x. this case we will fetch column name a, but hard to tell this a is from t or t2, so it will cause misunderstanding here, i will do it in next pr.

@ti-chi-bot ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 27, 2026
Copilot AI review requested due to automatic review settings January 27, 2026 03:49
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment on lines +758 to +820
func extractSelectIndexHints(sctx sessionctx.Context, defaultSchema string, node ast.StmtNode) []*indexHint {
selStmt, isSel := node.(*ast.SelectStmt)
if !isSel {
return nil
}
tableNames := extractSelectTableNamesWithAlias(defaultSchema, selStmt)
if len(tableNames) == 0 {
return nil
}
allowAnyTable := len(tableNames) == 1
hints := make([]*indexHint, 0)
seen := make(map[string]struct{})
for _, target := range tableNames {
extractor := &predicateColumnExtractor{
table: target,
columns: make(map[string]struct{}),
allowAnyTable: allowAnyTable,
}
if selStmt.Where != nil {
selStmt.Where.Accept(extractor)
}
if selStmt.From != nil && selStmt.From.TableRefs != nil {
collectJoinPredicates(selStmt.From.TableRefs, extractor)
}
if len(extractor.columns) == 0 {
continue
}
tblInfo, err := sctx.GetLatestInfoSchema().TableInfoByName(ast.NewCIStr(target.schema), ast.NewCIStr(target.name))
if err != nil {
continue
}
useInvisible := sctx.GetSessionVars().OptimizerUseInvisibleIndexes
for _, index := range tblInfo.Indices {
if index.State != model.StatePublic {
continue
}
if !useInvisible && index.Invisible {
continue
}
if index.IsColumnarIndex() || index.InvertedInfo != nil {
continue
}
if tblInfo.IsCommonHandle && index.Primary {
continue
}
if len(index.Columns) == 0 {
continue
}
if _, ok := extractor.columns[index.Columns[0].Name.L]; !ok {
continue
}
hint := &indexHint{
table: target,
index: index.Name.O,
}
if _, ok := seen[hint.String()]; ok {
continue
}
seen[hint.String()] = struct{}{}
hints = append(hints, hint)
}
}
return hints
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

extractSelectIndexHints currently generates index hints for any SELECT with table references in the FROM clause, including multi-table joins (it iterates over all tableNames regardless of length), while the PR description states that exploration should be limited to single-table SELECTs with predicates. Please either constrain this helper (for example by returning early when len(tableNames) != 1) or update the PR description to accurately describe that index hints are also applied to joined queries.

Copilot uses AI. Check for mistakes.
@ti-chi-bot ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jan 27, 2026
@AilinKid
Copy link
Contributor Author

/retest-required

@tiprow
Copy link

tiprow bot commented Jan 27, 2026

@AilinKid: PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test.

Details

In response to this:

/retest-required

Instructions 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.

@AilinKid
Copy link
Contributor Author

/ok-to-test

@ti-chi-bot ti-chi-bot bot added the ok-to-test Indicates a PR is ready to be tested. label Jan 27, 2026
@ti-chi-bot
Copy link

ti-chi-bot bot commented Jan 27, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: guo-shaoge, qw4990

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot added approved lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 27, 2026
@qw4990
Copy link
Contributor

qw4990 commented Jan 27, 2026

/retest

@ti-chi-bot
Copy link

ti-chi-bot bot commented Jan 27, 2026

[LGTM Timeline notifier]

Timeline:

  • 2026-01-27 04:55:10.493794623 +0000 UTC m=+1074538.107751478: ☑️ agreed by guo-shaoge.
  • 2026-01-27 10:02:47.656586785 +0000 UTC m=+1092995.270543641: ☑️ agreed by qw4990.

@AilinKid
Copy link
Contributor Author

/retest-required

1 similar comment
@AilinKid
Copy link
Contributor Author

/retest-required

@qw4990
Copy link
Contributor

qw4990 commented Jan 28, 2026

/retest

@AilinKid
Copy link
Contributor Author

/retest-required

@YangKeao
Copy link
Member

/retest

11 similar comments
@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@AilinKid
Copy link
Contributor Author

AilinKid commented Feb 1, 2026

/retest-required

Signed-off-by: AilinKid <314806019@qq.com>
Signed-off-by: AilinKid <314806019@qq.com>
Signed-off-by: AilinKid <314806019@qq.com>
Signed-off-by: AilinKid <314806019@qq.com>
@hawkingrei hawkingrei force-pushed the fix-plan-explore-index-hint branch from 83f102c to ca41e03 Compare February 2, 2026 14:14
@hawkingrei
Copy link
Member

/retest

@ti-chi-bot ti-chi-bot bot merged commit 53c8982 into pingcap:master Feb 2, 2026
30 checks passed
@AilinKid AilinKid deleted the fix-plan-explore-index-hint branch February 3, 2026 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm ok-to-test Indicates a PR is ready to be tested. release-note-none Denotes a PR that doesn't merit a release note. sig/planner SIG: Planner size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plan exploration can tell adjust index path choosing

5 participants