planner: add skew risk ratio for range pred by ichen032 · Pull Request #62035 · pingcap/tidb

ichen032 · 2025-06-26T20:00:37Z

What problem does this PR solve?

Issue Number: close #62093

Problem Summary:

What changed and how does it work?

This enhancement adds a new global/session variable tidb_opt_risk_range_skew_ratio that allows users or external processes to adjust how much skew is considered in our range estimation for ranges within a bucket.

When estimating a range predicate that is within a single histogram bucket, we assume even distribution of data within the bucket and so use the fraction of the bucket that our predicate covers * total row count for that bucket. Thus, we don't consider data skew which can be especially problematic with certain data types such as strings, where our data distribution typically has many gaps.

So, we calculate the worst case skew for a range to be the total row count of its bucket (excluding the repeat count of the last value if it is not within the range). Then we use our tidb_opt_risk_range_skew_ratio variable to determine how much of that skew is to be considered in our row estimation. For a default value of 0, there is no change to the original estimate, and when it is 1 , our row estimation = worst case skew.

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

ti-chi-bot · 2025-06-26T20:00:47Z

Hi @ichen032. Thanks for your PR.

I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

tiprow · 2025-06-26T20:00:53Z

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

terry1purcell · 2025-06-26T20:18:47Z

/ok-to-test

codecov · 2025-06-26T20:36:09Z

Codecov Report

Attention: Patch coverage is 88.46154% with 3 lines in your changes missing coverage. Please review.

Project coverage is 73.7781%. Comparing base (6e13f82) to head (7c1d4f3).
Report is 40 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #62035        +/-   ##
================================================
+ Coverage   72.9222%   73.7781%   +0.8559%     
================================================
  Files          1740       1781        +41     
  Lines        482965     503954     +20989     
================================================
+ Hits         352189     371808     +19619     
- Misses       109185     109645       +460     
- Partials      21591      22501       +910

Flag	Coverage Δ
integration	`45.3767% <61.5384%> (?)`
unit	`72.4465% <88.4615%> (+0.2816%)`	⬆️

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

Components	Coverage Δ
dumpling	`52.7804% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`45.7539% <ø> (-0.5691%)`	⬇️

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

terry1purcell · 2025-06-26T23:43:15Z

/retest-required

ichen032 · 2025-06-27T22:17:00Z

/retest

mjonss

Please add tests for the new variable.

That it works as expected.
That if it is set on Global level, it is inherited by Session level, and setting it on session level will override the global level.

ichen032 · 2025-07-01T22:44:03Z

/retest

terry1purcell · 2025-07-02T03:18:05Z

/retest-required

terry1purcell

/lgtm

mjonss

LGTM, only to minor formatting suggestions.

ti-chi-bot · 2025-07-04T09:41:58Z

[LGTM Timeline notifier]

Timeline:

2025-07-03 16:55:27.130345144 +0000 UTC m=+1586779.853524123: ☑️ agreed by terry1purcell.
2025-07-04 09:41:57.578296312 +0000 UTC m=+1647170.301475293: ☑️ agreed by mjonss.

Co-authored-by: Mattias Jonsson <mjonss@users.noreply.github.com>

ti-chi-bot · 2025-07-07T19:16:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: benmeadowcroft, mjonss, terry1purcell

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:

~~OWNERS~~ [mjonss]
~~pkg/planner/OWNERS~~ [terry1purcell]
~~pkg/sessionctx/vardef/OWNERS~~ [benmeadowcroft]
~~pkg/sessionctx/variable/OWNERS~~ [benmeadowcroft]
~~pkg/statistics/OWNERS~~ [terry1purcell]

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

…fix-scalar-func-init * commit '5ec4f36c855574b64be98e4c9f6b4485ceb4143d': (48 commits) dxf: use correct store in add-index, checksum by copr in import-into, and enable DXF service (pingcap#62224) infoschema: Add new fields in tiflash system table (pingcap#62296) lightning: fix panic on checksum manager close when checksum is off (pingcap#62300) *: Add owners cfg file for pkg/metrics module (pingcap#62292) autoid: reserve a table ID range for downstream fork (pingcap#62157) planner: Fix expression rewriting and method signature mismatch in plan cache (pingcap#58506) planner: use prop based noCopPushDown mechanism to replace aggregation field. (pingcap#62249) fix: close issue 59457 by trim compare first (pingcap#61915) planner: add skew risk ratio for range pred (pingcap#62035) meta: unify definition of system or memory DB name (pingcap#62247) ddl: add retry for updateSelfVersion (pingcap#62190) *: upgrade client go to add config valid function (pingcap#62246) executor: fix the inappropriate RequiredRows set by `TopNExec` (pingcap#62154) planner: RegardNULLAsPoint should be true as default (pingcap#62194) planner: apply predicate simplification before extract condition (pingcap#62211) test: fix failed test caused by new version of `mc` (pingcap#61356) planner: deprecate the logical interface CanPushToCop and its implementation canPushToCopImpl. (pingcap#62235) fix: shut down test without error (pingcap#61921) planner: lift the canPushToCop check of logical join/window/selection (pingcap#62206) backend/local: add rate limiter for split region and ingest data (pingcap#61555) ...

planner: add skew risk global var for range pred

47a809a

ti-chi-bot Bot added the sig/planner SIG: Planner label Jun 26, 2025

ichen032 closed this Jun 26, 2025

remove unrelated change

a434753

ichen032 reopened this Jun 26, 2025

ti-chi-bot Bot added ok-to-test Indicates a PR is ready to be tested. and removed needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Jun 26, 2025

terry1purcell self-requested a review June 26, 2025 20:22

hawkingrei reviewed Jun 27, 2025

View reviewed changes

Comment thread pkg/statistics/histogram.go Outdated

bug fixes

ee5a388

ti-chi-bot Bot removed the do-not-merge/needs-linked-issue label Jun 30, 2025

mjonss reviewed Jun 30, 2025

View reviewed changes

ichen032 added 2 commits July 1, 2025 12:01

update sctx requirements

a528802

add testcase for session/global

a8e2b05

ti-chi-bot Bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 1, 2025

ichen032 and others added 3 commits July 1, 2025 14:01

add bazel

79c7d1e

add bazel again

f48849f

Merge branch 'master' into rangeSkewRatio

f1c710d

terry1purcell requested a review from mjonss July 3, 2025 16:03

terry1purcell approved these changes Jul 3, 2025

View reviewed changes

ti-chi-bot Bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jul 3, 2025

mjonss approved these changes Jul 4, 2025

View reviewed changes

Comment thread pkg/statistics/histogram.go Outdated

Comment thread pkg/statistics/histogram.go Outdated

ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jul 4, 2025

terry1purcell and others added 2 commits July 7, 2025 11:57

Update pkg/statistics/histogram.go

2778341

Co-authored-by: Mattias Jonsson <mjonss@users.noreply.github.com>

Update pkg/statistics/histogram.go

7c1d4f3

Co-authored-by: Mattias Jonsson <mjonss@users.noreply.github.com>

benmeadowcroft approved these changes Jul 7, 2025

View reviewed changes

ti-chi-bot Bot added the approved label Jul 7, 2025

ti-chi-bot Bot merged commit 81370d4 into pingcap:master Jul 7, 2025
28 checks passed

Uh oh!

Conversation

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

Uh oh!

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Uh oh!

ti-chi-bot Bot commented Jun 26, 2025

Uh oh!

tiprow Bot commented Jun 26, 2025

Uh oh!

terry1purcell commented Jun 26, 2025

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

terry1purcell commented Jun 26, 2025

Uh oh!

Uh oh!

ichen032 commented Jun 27, 2025

Uh oh!

mjonss left a comment

Choose a reason for hiding this comment

Uh oh!

ichen032 commented Jul 1, 2025

Uh oh!

terry1purcell commented Jul 2, 2025

Uh oh!

terry1purcell left a comment

Choose a reason for hiding this comment

Uh oh!

mjonss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ti-chi-bot Bot commented Jul 4, 2025

[LGTM Timeline notifier]

Uh oh!

ti-chi-bot Bot commented Jul 7, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ichen032 commented Jun 26, 2025 •

edited

Loading

codecov Bot commented Jun 26, 2025 •

edited

Loading