statistics: split OutOfRangeRowCount into count-independent shape and scaling - #70178
statistics: split OutOfRangeRowCount into count-independent shape and scaling#70178terry1purcell wants to merge 3 commits into
Conversation
… scaling Refactor Histogram.OutOfRangeRowCount into two parts: - OutOfRangeShape computes the count-independent geometry of the out-of-range estimate (oneValue floor, histogram NDV, and the triangular-distribution overlap percentages), depending only on the histogram and the queried range. - ScaleOutOfRangeShape applies realtimeRowCount and modifyCount to a shape (determinate-mode handling, low-NDV oneValue adjustment, added-rows scaling, skew ratio, and min/max derivation). OutOfRangeRowCount keeps its signature as the composition of the two, so all call sites and estimates are unchanged. This separation allows a caller to compute and reuse the shape across changes in the realtime counts, a prerequisite for removing realtimeCount/modifyCount from the column estimate cache key introduced in pingcap#67098. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SyrcMzpvGA1rafdRP4m6HU
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesOut-of-range estimation
Estimated code review effort: 3 (Moderate) | ~25 minutes 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 #70178 +/- ##
================================================
- Coverage 76.3238% 74.0028% -2.3211%
================================================
Files 2041 2070 +29
Lines 559479 583664 +24185
================================================
+ Hits 427016 431928 +4912
- Misses 131563 150743 +19180
- Partials 900 993 +93
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 refactors out-of-range row-count estimation in pkg/statistics by separating the count-independent “shape” computation from the scaling step that applies realtimeRowCount / modifyCount, enabling callers to cache the geometry across changing realtime counts without changing estimation behavior.
Changes:
- Introduced
OutOfRangeShapeto capture the count-independent geometry of out-of-range estimation. - Refactored
Histogram.OutOfRangeRowCountintoOutOfRangeShape(...)+ScaleOutOfRangeShape(...)while keeping the original signature and behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SyrcMzpvGA1rafdRP4m6HU
|
/retest-required |
1 similar comment
|
/retest-required |
|
@terry1purcell: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
What problem does this PR solve?
Issue Number: ref #67097
Problem Summary:
Histogram.OutOfRangeRowCountmixes two concerns in one function: the geometry of the out-of-range estimate (derived only from the histogram and the queried range) and the scaling of that geometry byrealtimeRowCount/modifyCount. Because the function returns a row count already scaled to the realtime counts, any caller that wants to memoize out-of-range estimation must key its cache byrealtimeCountandmodifyCountas well (see the column estimate cache discussion in #67098). Coincidentally-equal counts would then risk incorrect cache reuse.What changed and how does it work?
Pure refactor of
Histogram.OutOfRangeRowCountinpkg/statistics/histogram.go, with no behavior change:OutOfRangeShape(new struct + method) computes the count-independent geometry: theoneValuefloor (NotNullCount/histNDV), the histogram NDV floored at 1, empty-histogram and impossible-range flags, and the triangular-distribution overlap percentages of the queried range with the out-of-range regions.ScaleOutOfRangeShape(new method) applies the currentrealtimeRowCountandmodifyCountto a shape: theOptObjectiveDeterminateearly return, the low-NDVoneValueadjustment, added-rows scaling, the risk range skew ratio, and the final min/est/max derivation.OutOfRangeRowCountkeeps its exact signature and is now the composition of the two, so all call sites (column and index estimation paths) and all estimates are unchanged.With this split, a follow-up can compute the shape once per (histogram, range) and rescale it as counts vary, which is the prerequisite for removing
realtimeCount/modifyCountfrom the column estimate cache key introduced in #67098.Check List
Tests
Covered by the existing out-of-range estimation suites, run with failpoints enabled:
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.
🤖 Generated with Claude Code
https://claude.ai/code/session_01SyrcMzpvGA1rafdRP4m6HU
Summary by CodeRabbit