planner: fix the wrong join estimation depending on missing or uninitialized stats #61604

qw4990 · 2025-06-09T11:41:37Z

What problem does this PR solve?

Issue Number: close #61602

Problem Summary: planner: fix the wrong join estimation depending on missing or uninitialized stats

What changed and how does it work?

planner: fix the wrong join estimation depending on missing or uninitialized stats

It's hard to construct test cases for this issue since it depends on stats cache's status. So I tested it locally, and this PR can work for this scenario:

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

Release note

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

None

tiprow · 2025-06-09T11:41:56Z

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

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.

pkg/planner/core/stats.go

hawkingrei · 2025-06-09T11:49:11Z

/retest

codecov · 2025-06-09T12:17:17Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.8131%. Comparing base (07e1f41) to head (975be8f).
Report is 55 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #61604        +/-   ##
================================================
+ Coverage   73.1044%   73.8131%   +0.7086%     
================================================
  Files          1729       1729                
  Lines        481039     484361      +3322     
================================================
+ Hits         351661     357522      +5861     
+ Misses       107839     105383      -2456     
+ Partials      21539      21456        -83

Flag	Coverage Δ
integration	`42.3901% <100.0000%> (?)`
unit	`72.6912% <100.0000%> (+0.3222%)`	⬆️

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

Components	Coverage Δ
dumpling	`52.7804% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`47.1658% <ø> (+0.2081%)`	⬆️

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

qw4990 · 2025-06-09T12:17:43Z

/retest

tiprow · 2025-06-09T12:18:04Z

@qw4990: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

elsa0520 · 2025-06-09T13:31:13Z

pkg/planner/core/stats.go

@@ -472,7 +472,7 @@ func getGroupNDVs(ds *logicalop.DataSource) []property.GroupNDV {
 					break
 				}
 			}
-			if match {


If there is no stats in the cache which means that it will return false in here. So what is the behavior of this situation ?

0xPoe · 2025-06-09T14:24:13Z

/cc

qw4990 · 2025-06-10T08:36:23Z

tests/integrationtest/r/executor/index_lookup_merge_join.result

-  ├─IndexRangeScan(Build)	10000.00	cop[tikv]	table:t2, index:PRIMARY(a, b, c)	range: decided by [eq(executor__index_lookup_merge_join.t2.a, executor__index_lookup_merge_join.t1.a) eq(executor__index_lookup_merge_join.t2.b, executor__index_lookup_merge_join.t1.b) eq(executor__index_lookup_merge_join.t2.c, executor__index_lookup_merge_join.t1.c)], keep order:false, stats:pseudo
-  └─TableRowIDScan(Probe)	10000.00	cop[tikv]	table:t2	keep order:false, stats:pseudo
+Sort	12500.00	root		executor__index_lookup_merge_join.t1.a:desc
+└─HashJoin	12500.00	root		left outer join, left side:TableReader, equal:[eq(executor__index_lookup_merge_join.t1.a, executor__index_lookup_merge_join.t2.a) eq(executor__index_lookup_merge_join.t1.c, executor__index_lookup_merge_join.t2.c) eq(executor__index_lookup_merge_join.t1.b, executor__index_lookup_merge_join.t2.b)]


12500 seems more reasonable, the prior 100000000.00 it not accurate.

0xPoe

Thanks!

0xPoe · 2025-06-10T21:10:00Z

pkg/planner/core/stats.go

@@ -472,7 +472,7 @@ func getGroupNDVs(ds *logicalop.DataSource) []property.GroupNDV {
 					break
 				}
 			}
-			if match {
+			if match && idx.IsEssentialStatsLoaded() {


Can we add some comments to explain why we need this extra check? Thanks!

qw4990 · 2025-06-11T10:41:32Z

/retest

tiprow · 2025-06-11T10:41:54Z

@qw4990: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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 · 2025-06-11T10:42:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AilinKid, hawkingrei

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [AilinKid,hawkingrei]
~~pkg/planner/OWNERS~~ [AilinKid,hawkingrei]

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 · 2025-06-11T10:42:39Z

[LGTM Timeline notifier]

Timeline:

2025-06-10 02:33:58.265942424 +0000 UTC m=+322416.494257684: ☑️ agreed by AilinKid.
2025-06-11 10:42:38.040751054 +0000 UTC m=+438136.269066318: ☑️ agreed by hawkingrei.

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot · 2025-06-11T11:47:04Z

In response to a cherrypick label: new pull request created to branch release-8.1: #61673.
But this PR has conflicts, please resolve them!

ti-chi-bot · 2025-06-11T11:47:48Z

In response to a cherrypick label: new pull request created to branch release-6.5: #61674.
But this PR has conflicts, please resolve them!

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot · 2025-06-11T11:48:34Z

In response to a cherrypick label: new pull request created to branch release-7.1: #61675.
But this PR has conflicts, please resolve them!

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot · 2025-06-11T11:49:19Z

In response to a cherrypick label: new pull request created to branch release-7.5: #61676.
But this PR has conflicts, please resolve them!

ti-chi-bot · 2025-06-19T23:43:37Z

In response to a cherrypick label: new pull request created to branch release-8.5: #61857.
But this PR has conflicts, please resolve them!

Signed-off-by: ti-chi-bot <[email protected]>

…ialized stats (#61604) (#61857) close #61602

…ialized stats (#61604) (#61676) close #61602

fixup

9b06e7a

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/needs-triage-completed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. sig/planner SIG: Planner labels Jun 9, 2025

ti-chi-bot bot removed the do-not-merge/needs-triage-completed label Jun 9, 2025

hawkingrei reviewed Jun 9, 2025

View reviewed changes

pkg/planner/core/stats.go Show resolved Hide resolved

elsa0520 reviewed Jun 9, 2025

View reviewed changes

ti-chi-bot bot requested a review from 0xPoe June 9, 2025 14:24

AilinKid approved these changes Jun 10, 2025

View reviewed changes

fixup

975be8f

ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 10, 2025

qw4990 commented Jun 10, 2025

View reviewed changes

0xPoe reviewed Jun 10, 2025

View reviewed changes

hawkingrei approved these changes Jun 11, 2025

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jun 11, 2025

ti-chi-bot bot merged commit 8d02f1f into pingcap:master Jun 11, 2025
24 checks passed

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Jun 11, 2025

This is an automated cherry-pick of pingcap#61604

d50d9f2

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request Jun 11, 2025

planner: fix the wrong join estimation depending on missing or uninitialized stats (#61604) #61673

Open

13 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Jun 11, 2025

This is an automated cherry-pick of pingcap#61604

09737dc

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request Jun 11, 2025

planner: fix the wrong join estimation depending on missing or uninitialized stats (#61604) #61674

Open

13 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Jun 11, 2025

This is an automated cherry-pick of pingcap#61604

6b8d114

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request Jun 11, 2025

planner: fix the wrong join estimation depending on missing or uninitialized stats (#61604) #61675

Open

13 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Jun 11, 2025

This is an automated cherry-pick of pingcap#61604

02c7233

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request Jun 11, 2025

planner: fix the wrong join estimation depending on missing or uninitialized stats (#61604) #61676

Merged

13 tasks

ti-chi-bot bot added the needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. label Jun 19, 2025

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Jun 19, 2025

This is an automated cherry-pick of pingcap#61604

f07922d

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request Jun 19, 2025

planner: fix the wrong join estimation depending on missing or uninitialized stats (#61604) #61857

Merged

13 tasks

ti-chi-bot bot pushed a commit that referenced this pull request Jul 9, 2025

planner: fix the wrong join estimation depending on missing or uninit…

9a0da87

…ialized stats (#61604) (#61857) close #61602

ti-chi-bot bot pushed a commit that referenced this pull request Jul 22, 2025

planner: fix the wrong join estimation depending on missing or uninit…

f6ee1fe

…ialized stats (#61604) (#61676) close #61602

planner: fix the wrong join estimation depending on missing or uninitialized stats #61604

planner: fix the wrong join estimation depending on missing or uninitialized stats #61604

Uh oh!

Conversation

qw4990 commented Jun 9, 2025

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Uh oh!

tiprow bot commented Jun 9, 2025

Uh oh!

Uh oh!

hawkingrei commented Jun 9, 2025

Uh oh!

codecov bot commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

qw4990 commented Jun 9, 2025

Uh oh!

tiprow bot commented Jun 9, 2025

Uh oh!

elsa0520 Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

0xPoe commented Jun 9, 2025

Uh oh!

qw4990 Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

0xPoe left a comment

Choose a reason for hiding this comment

Uh oh!

0xPoe Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

qw4990 commented Jun 11, 2025

Uh oh!

tiprow bot commented Jun 11, 2025

Uh oh!

ti-chi-bot bot commented Jun 11, 2025

Uh oh!

ti-chi-bot bot commented Jun 11, 2025

[LGTM Timeline notifier]

Uh oh!

Uh oh!

ti-chi-bot commented Jun 11, 2025

Uh oh!

ti-chi-bot commented Jun 11, 2025

Uh oh!

ti-chi-bot commented Jun 11, 2025

Uh oh!

ti-chi-bot commented Jun 11, 2025

Uh oh!

ti-chi-bot commented Jun 19, 2025

Uh oh!

Uh oh!

codecov bot commented Jun 9, 2025 •

edited

Loading