Skip to content

planner: fix the row count for index join use the PK in inner side#65190

Merged
ti-chi-bot[bot] merged 5 commits intopingcap:masterfrom
Reminiscent:indexjoin_inner_row_size
Jan 16, 2026
Merged

planner: fix the row count for index join use the PK in inner side#65190
ti-chi-bot[bot] merged 5 commits intopingcap:masterfrom
Reminiscent:indexjoin_inner_row_size

Conversation

@Reminiscent
Copy link
Copy Markdown
Contributor

@Reminiscent Reminiscent commented Dec 23, 2025

What problem does this PR solve?

Issue Number: close #65183

Problem Summary:
fix the row count for index join use the PK in inner side

What changed and how does it work?

Use the same way in the buildIndexJoinInner2IndexScan function(

maxOneRow := false
if indexJoinResult.chosenPath.Index.Unique && indexJoinResult.usedColsLen == len(indexJoinResult.chosenPath.FullIdxCols) {
l := len(indexJoinResult.chosenAccess)
if l == 0 {
maxOneRow = true
} else {
sf, ok := indexJoinResult.chosenAccess[l-1].(*expression.ScalarFunction)
maxOneRow = ok && (sf.FuncName.L == ast.EQ)
}
}
) which use to handle the unique index. It is very similar here. Check whether we use the full columns in the PK

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

@ti-chi-bot ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/needs-tests-checked size/L Denotes a PR that changes 100-499 lines, ignoring generated files. sig/planner SIG: Planner and removed do-not-merge/needs-tests-checked labels Dec 23, 2025
@tiprow
Copy link
Copy Markdown

tiprow bot commented Dec 23, 2025

Hi @Reminiscent. 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.

@Reminiscent
Copy link
Copy Markdown
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 Dec 23, 2025
@Reminiscent
Copy link
Copy Markdown
Contributor Author

@qw4990 @AilinKid PTAL

@Reminiscent Reminiscent marked this pull request as draft December 23, 2025 09:30
@ti-chi-bot ti-chi-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 23, 2025
@Reminiscent Reminiscent marked this pull request as ready for review December 23, 2025 09:30
@ti-chi-bot ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 23, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Dec 23, 2025

Codecov Report

❌ Patch coverage is 90.62500% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.6569%. Comparing base (4746c8d) to head (9cce572).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #65190        +/-   ##
================================================
- Coverage   77.8537%   77.6569%   -0.1968%     
================================================
  Files          1978       1902        -76     
  Lines        542181     529860     -12321     
================================================
- Hits         422108     411473     -10635     
+ Misses       118413     118381        -32     
+ Partials       1660          6      -1654     
Flag Coverage Δ
integration 41.5483% <75.0000%> (-6.6416%) ⬇️
unit 76.7753% <90.6250%> (+0.3092%) ⬆️

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

Components Coverage Δ
dumpling 56.7974% <ø> (ø)
parser ∅ <ø> (∅)
br 48.8378% <ø> (-12.2928%) ⬇️
🚀 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.

@Reminiscent
Copy link
Copy Markdown
Contributor Author

/retest-required

@hawkingrei
Copy link
Copy Markdown
Member

/retest

Comment on lines +977 to +986
maxOneRow := false
if indexJoinResult.chosenPath.Index.Unique && indexJoinResult.usedColsLen == len(indexJoinResult.chosenPath.FullIdxCols) {
l := len(indexJoinResult.chosenAccess)
if l == 0 {
maxOneRow = true
} else {
sf, ok := indexJoinResult.chosenAccess[l-1].(*expression.ScalarFunction)
maxOneRow = ok && (sf.FuncName.L == ast.EQ)
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we wrap this part of code as a separate function? Then we can avoid some duplicated code.

@ti-chi-bot ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 13, 2026
indexJoinResult *indexJoinPathResult) (rangeInfo string, maxOneRow bool) {
rangeInfo = indexJoinPathRangeInfo(sctx, outerJoinKeys, indexJoinResult)
maxOneRow = false
if indexJoinResult.chosenPath.Index.Unique && indexJoinResult.usedColsLen == len(indexJoinResult.chosenPath.FullIdxCols) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

make sense

@AilinKid
Copy link
Copy Markdown
Contributor

/review

@ti-chi-bot ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 14, 2026
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot bot commented Jan 14, 2026

[LGTM Timeline notifier]

Timeline:

  • 2026-01-13 10:21:09.924639201 +0000 UTC m=+352913.986504113: ☑️ agreed by qw4990.
  • 2026-01-14 05:42:57.324982429 +0000 UTC m=+422621.386847364: ☑️ agreed by AilinKid.

@AilinKid
Copy link
Copy Markdown
Contributor

/retest-required

@Reminiscent
Copy link
Copy Markdown
Contributor Author

/retest

@ti-chi-bot ti-chi-bot bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 14, 2026
@ti-chi-bot ti-chi-bot bot removed the approved label Jan 14, 2026
@Reminiscent Reminiscent force-pushed the indexjoin_inner_row_size branch from a3ed703 to e60bf2e Compare January 14, 2026 09:56
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot bot commented Jan 14, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AilinKid, 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 size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 14, 2026
@Reminiscent Reminiscent force-pushed the indexjoin_inner_row_size branch from e60bf2e to 938a0db Compare January 14, 2026 09:58
@Reminiscent
Copy link
Copy Markdown
Contributor Author

/retest

4 similar comments
@hawkingrei
Copy link
Copy Markdown
Member

/retest

@Reminiscent
Copy link
Copy Markdown
Contributor Author

/retest

@Reminiscent
Copy link
Copy Markdown
Contributor Author

/retest

@Reminiscent
Copy link
Copy Markdown
Contributor Author

/retest

@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 15, 2026
@Reminiscent Reminiscent force-pushed the indexjoin_inner_row_size branch from c44d22c to 9cce572 Compare January 16, 2026 02:16
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot bot commented Jan 16, 2026

@Reminiscent: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-br-integration-test a3ed703 link true /test pull-br-integration-test
pull-lightning-integration-test a3ed703 link true /test pull-lightning-integration-test

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@Reminiscent
Copy link
Copy Markdown
Contributor Author

/test mysql-test

@tiprow
Copy link
Copy Markdown

tiprow bot commented Jan 16, 2026

@Reminiscent: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

Details

In response to this:

/test mysql-test

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.

@ti-chi-bot ti-chi-bot bot merged commit f8627fd into pingcap:master Jan 16, 2026
30 of 31 checks passed
@Reminiscent Reminiscent deleted the indexjoin_inner_row_size branch January 16, 2026 03:36
@Reminiscent
Copy link
Copy Markdown
Contributor Author

/cherrypick release-8.5

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Jan 23, 2026
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Copy Markdown
Member

@Reminiscent: new pull request created to branch release-8.5: #65745.
But this PR has conflicts, please resolve them!

Details

In response to this:

/cherrypick release-8.5

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 ti-community-infra/tichi repository.

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

Labels

approved component/statistics 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.

planner: Row count incorrectly forced to 1 for IndexJoin when matching only a prefix of a composite Primary Key

5 participants