planner: credit appended handle predicates in index row count estimation#69749
planner: credit appended handle predicates in index row count estimation#69749terry1purcell wants to merge 8 commits into
Conversation
pruneEstimateRange truncates ranges built over the index columns plus the appended handle columns down to the declared index columns so they align with index statistics. The truncation kept the original exclusion flags and estimated every pruned range independently, which breaks in two ways once a handle predicate extends the range: - An exclusive bound whose values were dropped collapses the range to empty: a = 5 AND id > 10 builds (5 10, 5 +inf], which pruned to (5, 5] and estimated ~0 rows regardless of the selectivity of a = 5. - Multiple handle points under one prefix double count: a = 5 AND id IN (11, 22) pruned to [5,5], [5,5] and counted the a = 5 rows twice. Make a truncated bound inclusive, since dropping trailing dimensions widens it to the whole prefix, and merge the pruned ranges with ranger.UnionRanges so overlapping prefixes are counted once. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
For a non-unique secondary index, predicates on the handle columns become part of the execution ranges, but the path's CountAfterAccess is estimated from ranges pruned back to the declared index columns and gave those predicates no credit: a path whose benefit is the handle seek looked as expensive as one without it. Damp the pruned prefix estimate with the handle columns' selectivities using the exponential backoff series - the prefix keeps the full-weight slot and each handle column contributes sel^(1/2), sel^(1/4), ... from the most selective one - so the handle predicates tighten the estimate without assuming independence between the index columns and the primary key. A range that point-binds the declared columns plus the full handle matches at most one row because the physical key is unique, so such estimates are additionally capped by the number of ranges. Selectivity() already credits these predicates through exponential backoff, so when the damped CountAfterAccess falls below stats.RowCount, align it upward to stats.RowCount without the SelectionFactor penalty: the shortfall is expected damping, not the inconsistent-assumption case the penalty exists for. Non-point handle predicates currently receive no net credit after this alignment because expBackoffEstimation floors its result at 1/NDV of the declared index columns; lifting that floor needs the multi-column histogram bounds to be truncated first and is left as a follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ti-chi-bot
Bot
added
release-note
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe planner now credits appended handle predicates with damped selectivity, preserves corrected truncated ranges, aligns access counts, and caps full-point estimates. Tests cover exclusive, point, and unsigned clustered-handle predicates. ChangesAppended Handle Cardinality
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Query as Query predicates
participant Planner as Planner range estimation
participant Stats as Column and index statistics
participant Path as Index access path
Query->>Planner: Build index ranges
Planner->>Stats: Estimate declared-column prefix
Stats-->>Planner: Prefix row estimate
Planner->>Stats: Estimate appended-handle selectivity
Stats-->>Planner: Damped handle adjustment
Planner->>Path: Store adjusted access counts
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #69749 +/- ##
================================================
- Coverage 76.3206% 73.9943% -2.3264%
================================================
Files 2041 2057 +16
Lines 559973 578424 +18451
================================================
+ Hits 427375 428001 +626
- Misses 131697 150087 +18390
+ Partials 901 336 -565
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR improves TiDB planner cardinality estimation for non-unique secondary index access paths whose execution ranges include appended handle (primary key) columns, so that handle predicates influence CountAfterAccess and path costing more accurately (and avoids the SelectionFactor penalty when the damped estimate falls below stats.RowCount).
Changes:
- Add appended-handle-aware row-count damping (
AdjustRowCountForAppendedHandleColumns) using exponential backoff over handle-column selectivities, with a cap for full-point (unique physical key) ranges. - Update index-range estimation flow to prune/union estimation ranges with a
RangerContext, and adjustadjustCountAfterAccessbehavior when ranges include appended handle columns. - Add unit tests covering handle-extended ranges (exclusive bounds, IN-list point ranges, and full point lookups), and update Bazel deps.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/planner/core/stats.go | Detects handle-extended ranges, prunes/merges estimation ranges with RangerContext, applies appended-handle damping, and avoids SelectionFactor penalty when aligning to stats.RowCount for handle-extended ranges. |
| pkg/planner/core/BUILD.bazel | Adds //pkg/util/ranger/context dependency to support the new pruning/union API usage. |
| pkg/planner/cardinality/selectivity_test.go | Adds a regression/unit test validating estimation behavior for truncated/handle-extended index ranges and point-range capping. |
| pkg/planner/cardinality/row_count_index.go | Introduces AdjustRowCountForAppendedHandleColumns to credit appended handle predicates via exponential backoff and cap full-point ranges. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| sctx planctx.PlanContext, | ||
| coll *statistics.HistColl, | ||
| ranges []*ranger.Range, | ||
| idxCols []*expression.Column, |
There was a problem hiding this comment.
How about renaming this argument to "appendedIdxCols" to represent that it contains both the declared index columns and PK columns?
There was a problem hiding this comment.
Agreed the name should distinguish this from the idxCols params elsewhere in this file, which receive only the declared columns. My only hesitation with appendedIdxCols is it could be read as containing only the appended handle columns. How about idxColsWithHandle? (I would avoid fullIdxCols since AccessPath.FullIdxCols already means the declared columns without the handle.) Renamed to idxColsWithHandle — happy to adjust if you prefer another name.
| // handle and is therefore unique. | ||
| fullPoints := true | ||
| for _, ran := range ranges { | ||
| if len(ran.LowVal) != len(idxCols) || len(ran.HighVal) != len(idxCols) || |
There was a problem hiding this comment.
It seems like there is an implicit assumption: the idxCols contains all PK columns? Could we add a comment for this implicit assumption?
There was a problem hiding this comment.
Good catch — yes, the cap is only valid because the appended dimensions cover the complete handle. That is guaranteed by fillIndexPath, which appends the handle only when the primary key is the single integer handle column (GetPKIsHandleCol), so the appended suffix always identifies a row exactly. Added a comment documenting this contract on the function doc and at the full-point cap.
ti-chi-bot
Bot
added
approved
needs-1-more-lgtm
Rename the idxCols parameter of AdjustRowCountForAppendedHandleColumns to idxColsWithHandle to distinguish it from the declared-columns-only idxCols parameters elsewhere in this file, and document the contract that the appended dimensions cover the complete integer handle, which the full-point one-row cap relies on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
winoros
left a comment
winoros
left a comment
There was a problem hiding this comment.
There's a special fact in TiDB:
TiKV stores the unsigned int pk in a wrong physical order.
It stores the part [MaxInt64+1, MaxUint64] before [0, MaxInt64]. But statistics doesn't handle this. I think we need to check the behavior and test it.
… estimation Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Thank you. Upon investigation - this is exactly why fillIndexPath has never appended an unsigned handle (!HasUnsignedFlag guard, predates this PR), so the new credit/prune code can't fire for unsigned PKs. The latest commit added an unsigned-PK case to the test to pin that down. |
# Conflicts: # pkg/planner/core/stats.go
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qw4990, winoros The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What problem does this PR solve?
Issue Number: close #69748
Problem Summary:
For a non-unique secondary index, predicates on the handle columns become part of the execution ranges (the physical key ends with the handle), but the path's
CountAfterAccessis estimated from ranges pruned back to the declared index columns and gives those predicates no credit.a = 5 AND id = 7onKEY ia(a)builds the point range[5 7,5 7], which matches at most one row because the physical key(a, id)is unique — yet it estimates the fulla = 5prefix count. A path whose benefit is the handle seek looks as expensive as one without it and can lose path selection.Worse,
Selectivity()does credit these predicates (through exponential backoff overIdx2ColUniqueIDs, which includes the appended handle), so any attempt to creditCountAfterAccessthat lands belowstats.RowCountwas treated byadjustCountAfterAccessas inconsistent assumptions and penalized with1/SelectionFactor— making the credited path more expensive (12.50 instead of 10.00 in the test scenario).Depends on #69747 (this branch stacks on it; will rebase once it merges).
What changed and how does it work?
New
cardinality.AdjustRowCountForAppendedHandleColumns, applied indetachCondAndBuildRangeForPathafter the pruned-prefix estimate:sel^(1/2), sel^(1/4), ...starting from the most selective, using its column statistics. This credits the handle predicates without assuming independence between the index columns and the primary key.ranger.UnionRangesfirst, so ranges differing only in earlier dimensions do not double count.Consistency with
Selectivity(): when the dampedCountAfterAccessfalls belowstats.RowCount,adjustCountAfterAccessnow aligns it upward tostats.RowCountwithout theSelectionFactorpenalty for handle-extended ranges — the shortfall is expected damping, not the inconsistent-assumption case the penalty exists for.Resulting estimates on the test table (100 rows, 10 per value of
a, previously all 10.00):a = 5 and id = 7a = 5 and id in (11, 22)a = 5 and id > 10Non-point handle predicates receive no net credit yet:
expBackoffEstimationfloors its result at1/NDVof the declared index columns, so the alignment brings them back to the prefix estimate. Lifting that floor requires truncating the full-key bounds used by the multi-column histogram upper-limit computation and is left to a follow-up (see the issue).Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.
Summary by CodeRabbit