[DNM] planner, statistics: use selected partition stats for dynamic pruning#67139
Closed
Reminiscent wants to merge 2 commits into
Closed
[DNM] planner, statistics: use selected partition stats for dynamic pruning#67139Reminiscent wants to merge 2 commits into
Reminiscent wants to merge 2 commits into
Conversation
ti-chi-bot
Bot
added
release-note-none
|
This cherry pick PR is for a release branch and has not yet been approved by triage owners. To merge this cherry pick:
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. |
ti-chi-bot
Bot
added
size/XXL
📝 WalkthroughWalkthroughThis PR adds selective partition-statistics support for dynamic partition pruning, introduces session/sysvar controls, tracks prepared-plan build and static-prune states, aggregates per-partition counts, adjusts planner/pruning/plan-cache behavior to avoid unsafe cached plans, and adds/updates tests and BUILD shards. Changes
Sequence Diagram(s)sequenceDiagram
participant S as Session/Vars
participant P as Planner/Builder
participant PP as PartitionProcessor
participant SC as StatsCollector
participant PC as PlanCache
S->>P: Build plan (EnableSelectedPartitionStats)
P->>P: Set InPreparedPlanBuild=true (for prepared build)
P->>PP: Request pruning
PP-->>P: StaticPrunedPartitionIDs (or nil) + pruned result
P->>SC: Request per-partition stats when StaticPrunedPartitionIDs present
SC-->>P: Partition stats
P->>P: AggregateSelectedPartitionCounts -> override RealtimeCount if ok
P->>PC: Query cached plan
PC->>PC: shouldSkipCachedPlanForStaticPartitionPruning?
alt skip
PC-->>S: Skip cache (SetSkipPlanCache)
else use
PC-->>S: Return cached plan
end
S->>P: Execute final plan
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
pkg/planner/core/plan_cache_utils.go (1)
175-182: Document the temporaryStmtCtxoverride.The save/reset/restore around
destBuilder.Buildis a non-obvious planner invariant. A short comment here would make future edits much less error-prone.As per coding guidelines, "Comments SHOULD explain non-obvious intent, constraints, invariants, concurrency guarantees, SQL/compatibility contracts, or important performance trade-offs, and SHOULD NOT restate what the code already makes clear"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/planner/core/plan_cache_utils.go` around lines 175 - 182, Add a short clarifying comment above the block that temporarily saves and overrides vars.StmtCtx.InPreparedPlanBuild and vars.StmtCtx.StaticPartitionPrune around the call to destBuilder.Build(ctx, nodeW), explaining that the planner intentionally sets InPreparedPlanBuild (to isPrepStmt && cacheable) and disables StaticPartitionPrune only for the duration of building this prepared plan node and must restore the previous values afterward to preserve planner invariants for subsequent planning; reference the specific symbols vars.StmtCtx.InPreparedPlanBuild, vars.StmtCtx.StaticPartitionPrune, destBuilder.Build and nodeW to make the purpose and necessity of the save/restore explicit.pkg/planner/core/casetest/planstats/plan_stats_test.go (1)
641-642: Consider avoiding a root-node hard cast to*PhysicalTableReaderin this test.At Line 641, strict root type assertion can make the test fragile to benign optimizer shape changes. A recursive lookup for the first table reader would keep the test focused on stats semantics.
Possible refactor
- reader, ok := plan.(*plannercore.PhysicalTableReader) - require.True(t, ok) + reader := findFirstTableReader(plan) + require.NotNil(t, reader) require.Equal(t, tc.expectedRowCount, reader.StatsInfo().HistColl.RealtimeCount) } } + +func findFirstTableReader(p base.Plan) *plannercore.PhysicalTableReader { + if r, ok := p.(*plannercore.PhysicalTableReader); ok { + return r + } + pp, ok := p.(base.PhysicalPlan) + if !ok { + return nil + } + for _, child := range pp.Children() { + if r := findFirstTableReader(child); r != nil { + return r + } + } + return nil +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/planner/core/casetest/planstats/plan_stats_test.go` around lines 641 - 642, The test currently does a brittle root-node type assertion (reader, ok := plan.(*plannercore.PhysicalTableReader)); instead, implement a small recursive/walk helper that traverses the plan tree and returns the first node of type *plannercore.PhysicalTableReader (e.g., findFirstPhysicalTableReader(plan plannercore.Plan) or walkPlanNodes) and use that result in the assertions; this keeps the test resilient to harmless plan-shape/optimizer changes while still validating the reader's statistics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/planner/core/logical_plan_builder.go`:
- Around line 4557-4559: The current guard using hasGlobalIndex around the
dynamic-prune fallback causes the static-prune fallback to be skipped for
partitioned tables with global indexes; change the logic so that the
static-prune fallback (the code that disables dynamic pruning and sets
StmtCtx.UseDynamicPruneMode = false and emits the warning) always runs when
global stats are missing, while only gating the selected-partition-stats reuse
branch behind hasGlobalIndex; specifically update the block around
slices.ContainsFunc(tableInfo.Indices, ...) / hasGlobalIndex so it only
influences the reuse path (the code in the selected-partition-stats branch) and
not the fallback that flips StmtCtx.UseDynamicPruneMode, and add a planner unit
test/regression for a partitioned table with a global index and missing global
stats (and update related rule testdata as needed, also apply the same fix for
the other occurrence around lines 4587-4595).
- Around line 4223-4230: The current logic reuses
statsHandle.GetPhysicalTableStats(tblInfo.ID, tblInfo) and only overrides
RealtimeCount/ModifyCount, which causes the merged selected-partition stats to
inherit the global table's pseudo/uninitialized state; instead, when
cardinality.AggregateSelectedPartitionCounts returns ok, build statsTbl from the
selected partitions' merged stats (don’t start from
GetPhysicalTableStats(tblInfo.ID,...)), e.g. obtain or construct a TableStats
object that reflects the selected partitions' initialization/state and set
RealtimeCount and ModifyCount from realtimeCount/modifyCount, then set
selectedPartitionCountsOverridden = true so the optimizer uses the partitions'
real initialization status rather than the global pseudo one.
In `@pkg/planner/core/plan_cache_utils.go`:
- Around line 187-190: The warning text is incorrect for the non-prepared
branch: when cacheable && !isPrepStmt && staticPartitionPrune is true you
currently append "skip prepared plan-cache" which misleads SHOW WARNINGS; update
the message produced in sctx.GetSessionVars().StmtCtx.AppendWarning(...) to use
the non-prepared cache warning prefix (e.g., "skip non-prepared plan-cache:
"+reason or the existing non-prepared prefix used elsewhere) so the warning
matches the condition where isPrepStmt is false and reason is "static partition
prune mode used".
In `@pkg/planner/core/plan_cache.go`:
- Around line 384-386: The helper currently treats an access object with a
populated accessObj.err as a non-error by returning (false, nil); instead, when
getDynamicAccessPartition(sctx, tblInfo, partInfo, "") yields accessObj != nil
and accessObj.err is non-empty, stop treating the partition resolution as a
cacheable hit and propagate the error (return false, accessObj.err or wrap and
return that error) so callers know partition resolution failed; update the
branch that checks accessObj and accessObj.err to return a non-nil error rather
than nil.
- Around line 297-317: The current skip logic in
shouldSkipCachedPlanForStaticPartitionPruning (and the similar predicates at the
other noted locations) uses the observed partition subset size to decide to skip
cached plans; change it to detect whether partition pruning is runtime-dependent
instead of checking "fewer than all partitions". Specifically, update
cachedPlanUsesStaticPartitionPruning (and callers like
shouldSkipCachedPlanForStaticPartitionPruning, and the analogous predicates at
the other spots) to return true only when pruning decisions depend on runtime
values (e.g., expressions or parameters evaluated at execution) and return false
when the partition set is statically fixed by the statement/schema (such as
PARTITION(...)). Ensure the guard uses that runtime-dependent flag to skip cache
reuse and leaves cached plans usable for fixed/static partition subsets.
In `@pkg/planner/core/rule_partition_processor.go`:
- Around line 1917-1932: The computation of staticPruned currently uses
len(prunedPartitionIDs) < len(pi.Definitions) which can be true for fixed
PARTITION(...) or dropping/overlapping partitions even when runtime predicates
pruned nothing; change the check that sets staticPruned (and the subsequent
calls to setStaticPartitionPruneInfo and setting ds.StaticPrunedPartitionIDs) to
compare prunedPartitionIDs against the post-filter candidate baseline (the
candidate set after applying PartitionNames/non-dropping logic) rather than
pi.Definitions. Locate the staticPruned assignment and replace its baseline with
the post-PartitionNames/non-dropping candidate count (or an explicitly tracked
baseline variable computed earlier), ensuring the rest of the branch that uses
staticPruned (the calls to setStaticPartitionPruneInfo, the
UseDynamicPruneMode/EnableSelectedPartitionStats guards, and
ds.StaticPrunedPartitionIDs) only run when runtime predicate pruning actually
removed partitions beyond that baseline.
---
Nitpick comments:
In `@pkg/planner/core/casetest/planstats/plan_stats_test.go`:
- Around line 641-642: The test currently does a brittle root-node type
assertion (reader, ok := plan.(*plannercore.PhysicalTableReader)); instead,
implement a small recursive/walk helper that traverses the plan tree and returns
the first node of type *plannercore.PhysicalTableReader (e.g.,
findFirstPhysicalTableReader(plan plannercore.Plan) or walkPlanNodes) and use
that result in the assertions; this keeps the test resilient to harmless
plan-shape/optimizer changes while still validating the reader's statistics.
In `@pkg/planner/core/plan_cache_utils.go`:
- Around line 175-182: Add a short clarifying comment above the block that
temporarily saves and overrides vars.StmtCtx.InPreparedPlanBuild and
vars.StmtCtx.StaticPartitionPrune around the call to destBuilder.Build(ctx,
nodeW), explaining that the planner intentionally sets InPreparedPlanBuild (to
isPrepStmt && cacheable) and disables StaticPartitionPrune only for the duration
of building this prepared plan node and must restore the previous values
afterward to preserve planner invariants for subsequent planning; reference the
specific symbols vars.StmtCtx.InPreparedPlanBuild,
vars.StmtCtx.StaticPartitionPrune, destBuilder.Build and nodeW to make the
purpose and necessity of the save/restore explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fe448d13-8e63-4fbb-9b52-9721531321a6
📒 Files selected for processing (19)
pkg/planner/cardinality/BUILD.bazelpkg/planner/cardinality/pseudo.gopkg/planner/core/casetest/instanceplancache/others_test.gopkg/planner/core/casetest/planstats/plan_stats_test.gopkg/planner/core/collect_column_stats_usage.gopkg/planner/core/logical_plan_builder.gopkg/planner/core/operator/logicalop/logical_datasource.gopkg/planner/core/plan_cache.gopkg/planner/core/plan_cache_partition_table_test.gopkg/planner/core/plan_cache_utils.gopkg/planner/core/planbuilder.gopkg/planner/core/rule_partition_processor.gopkg/planner/core/stats.gopkg/planner/core/tests/prepare/prepare_test.gopkg/sessionctx/stmtctx/stmtctx.gopkg/sessionctx/variable/session.gopkg/sessionctx/variable/setvar_affect.gopkg/sessionctx/variable/sysvar.gopkg/sessionctx/variable/tidb_vars.go
Comment on lines
+4223
to
+4230
| // Reuse the global stats objects and only narrow the table-level counts to the selected partitions. | ||
| statsTbl = statsHandle.GetPhysicalTableStats(tblInfo.ID, tblInfo) | ||
| if realtimeCount, modifyCount, ok := cardinality.AggregateSelectedPartitionCounts(uniquePartitionStats); ok { | ||
| statsTbl = statsTbl.ShallowCopy() | ||
| statsTbl.RealtimeCount = realtimeCount | ||
| statsTbl.ModifyCount = modifyCount | ||
| selectedPartitionCountsOverridden = true | ||
| } |
There was a problem hiding this comment.
Don't anchor merged selected-partition stats on a pseudo global table.
For the multi-partition case, this always starts from tblInfo.ID stats and only overrides the counts. If the global table stats are uninitialized/pseudo while the selected partitions all have non-pseudo stats, the later pseudo checks still win and the optimizer drops back to pseudo stats again. The merged result needs to come from the selected partition stats themselves instead of inheriting the global table's initialization state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/planner/core/logical_plan_builder.go` around lines 4223 - 4230, The
current logic reuses statsHandle.GetPhysicalTableStats(tblInfo.ID, tblInfo) and
only overrides RealtimeCount/ModifyCount, which causes the merged
selected-partition stats to inherit the global table's pseudo/uninitialized
state; instead, when cardinality.AggregateSelectedPartitionCounts returns ok,
build statsTbl from the selected partitions' merged stats (don’t start from
GetPhysicalTableStats(tblInfo.ID,...)), e.g. obtain or construct a TableStats
object that reflects the selected partitions' initialization/state and set
RealtimeCount and ModifyCount from realtimeCount/modifyCount, then set
selectedPartitionCountsOverridden = true so the optimizer uses the partitions'
real initialization status rather than the global pseudo one.
Comment on lines
+297
to
+317
| func shouldSkipCachedPlanForStaticPartitionPruning(sctx sessionctx.Context, plan base.Plan) (bool, error) { | ||
| if !sctx.GetSessionVars().StmtCtx.UseDynamicPartitionPrune() || !sctx.GetSessionVars().EnableSelectedPartitionStats { | ||
| return false, nil | ||
| } | ||
| switch p := plan.(type) { | ||
| case *Update: | ||
| if p.SelectPlan == nil { | ||
| return false, nil | ||
| } | ||
| return cachedPlanUsesStaticPartitionPruning(sctx, p.SelectPlan) | ||
| case *Delete: | ||
| if p.SelectPlan == nil { | ||
| return false, nil | ||
| } | ||
| return cachedPlanUsesStaticPartitionPruning(sctx, p.SelectPlan) | ||
| case base.PhysicalPlan: | ||
| return cachedPlanUsesStaticPartitionPruning(sctx, p) | ||
| default: | ||
| return false, nil | ||
| } | ||
| } |
There was a problem hiding this comment.
Base the cache-skip decision on runtime pruning, not subset size.
This guard now skips reuse whenever the execution touches fewer than all partitions. That also matches fixed subsets from PARTITION (...) and other invariant partition restrictions, where the partition set is part of the statement/schema and does not vary per execution. Those statements will miss plan cache on every reuse even though the cached plan is safe. The skip predicate needs to distinguish runtime-dependent pruning from a fixed partition set.
Also applies to: 380-389, 414-418, 446-458
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/planner/core/plan_cache.go` around lines 297 - 317, The current skip
logic in shouldSkipCachedPlanForStaticPartitionPruning (and the similar
predicates at the other noted locations) uses the observed partition subset size
to decide to skip cached plans; change it to detect whether partition pruning is
runtime-dependent instead of checking "fewer than all partitions". Specifically,
update cachedPlanUsesStaticPartitionPruning (and callers like
shouldSkipCachedPlanForStaticPartitionPruning, and the analogous predicates at
the other spots) to return true only when pruning decisions depend on runtime
values (e.g., expressions or parameters evaluated at execution) and return false
when the partition set is statically fixed by the statement/schema (such as
PARTITION(...)). Ensure the guard uses that runtime-dependent flag to skip cache
reuse and leaves cached plans usable for fixed/static partition subsets.
Comment on lines
+384
to
+386
| accessObj := getDynamicAccessPartition(sctx, tblInfo, partInfo, "") | ||
| if accessObj == nil || len(accessObj.err) > 0 { | ||
| return false, nil |
There was a problem hiding this comment.
Don't turn partition-resolution failures into cache hits.
If getDynamicAccessPartition() returns an access object with err populated, this helper currently returns (false, nil) and lets cached-plan reuse continue. Since this function is the safety gate, an unresolved partition set should conservatively skip cache (or be surfaced), not fall through as reusable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/planner/core/plan_cache.go` around lines 384 - 386, The helper currently
treats an access object with a populated accessObj.err as a non-error by
returning (false, nil); instead, when getDynamicAccessPartition(sctx, tblInfo,
partInfo, "") yields accessObj != nil and accessObj.err is non-empty, stop
treating the partition resolution as a cacheable hit and propagate the error
(return false, accessObj.err or wrap and return that error) so callers know
partition resolution failed; update the branch that checks accessObj and
accessObj.err to return a non-nil error rather than nil.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release-8.5 #67139 +/- ##
================================================
Coverage ? 55.0291%
================================================
Files ? 1823
Lines ? 655195
Branches ? 0
================================================
Hits ? 360548
Misses ? 267774
Partials ? 26873
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Keep the release-8.5 selected-partition-stats backport compatible with existing planner behavior while preserving the new feature where it is safe. This commit: - adds the partition-count aggregation unit test and updates Bazel metadata for the new tests - keeps explicit PARTITION() queries cacheable while still rejecting runtime-sensitive partition pruning for cached plans - preserves the legacy dynamic-pruning fallback when global stats are missing, including partitioned tables with global indexes - avoids adding ExtraPhysTblID to static-pruning UnionScan plans so default-off behavior does not change existing plan shapes - falls back to the legacy global/pseudo stats path for multi-partition queries when table-level global stats are still pseudo or uninitialized - adds regression tests for missing global stats, explicit partition plan-cache reuse, and selected partition stats behavior
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/planner/cardinality/ndv_test.go (1)
35-39: Assert returned counts on the failure path as well.The pseudo case currently checks only
ok. Adding count assertions locks in the(0, 0, false)contract and catches regressions earlier.📌 Suggested test tightening
- _, _, ok = AggregateSelectedPartitionCounts([]*statistics.Table{ + realtimeCount, modifyCount, ok = AggregateSelectedPartitionCounts([]*statistics.Table{ {HistColl: *statistics.NewHistColl(1, true, 10, 2, 0, 0)}, pseudoStats, }) require.False(t, ok) + require.Equal(t, int64(0), realtimeCount) + require.Equal(t, int64(0), modifyCount)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/planner/cardinality/ndv_test.go` around lines 35 - 39, The test calls AggregateSelectedPartitionCounts but only asserts the boolean `ok`; update the call to capture the two returned counts (e.g., c1, c2, ok := AggregateSelectedPartitionCounts(...)) and add assertions that c1 and c2 equal 0 when ok is false (use require.Equal or similar) to lock in the (0, 0, false) contract — reference the AggregateSelectedPartitionCounts call in ndv_test.go and the existing require.False assertion to replace with count checks plus require.False.pkg/planner/core/plan_cache_partition_table_test.go (1)
364-365: Consider defensive check for warning presence.Lines 365 and 375 unconditionally access
Rows()[0][2], which will panic if no warnings are returned. Other tests in this file use defensive checks like:if warnings := tk.MustQuery(`show warnings`).Rows(); len(warnings) > 0 { requireStaticPartitionPruneWarning(t, warnings[0][2].(string)) }If the assertion is that warnings must be present, consider using
require.NotEmptyfirst for a clearer failure message:warnings := tk.MustQuery(`show warnings`).Rows() require.NotEmpty(t, warnings, "expected static partition prune warning") requireStaticPartitionPruneWarning(t, warnings[0][2].(string))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/planner/core/plan_cache_partition_table_test.go` around lines 364 - 365, The test unconditionally indexes show warnings result which can panic if empty; change the two places that access tk.MustQuery(`show warnings`).Rows()[0][2] to first capture warnings := tk.MustQuery(`show warnings`).Rows(), assert they are present with require.NotEmpty(t, warnings, "expected static partition prune warning") (or use len check like other tests), then call requireStaticPartitionPruneWarning(t, warnings[0][2].(string)); update references around the assertions involving tk.Session().GetSessionVars().FoundInPlanCache and requireStaticPartitionPruneWarning to use this defensive check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/planner/cardinality/ndv_test.go`:
- Around line 35-39: The test calls AggregateSelectedPartitionCounts but only
asserts the boolean `ok`; update the call to capture the two returned counts
(e.g., c1, c2, ok := AggregateSelectedPartitionCounts(...)) and add assertions
that c1 and c2 equal 0 when ok is false (use require.Equal or similar) to lock
in the (0, 0, false) contract — reference the AggregateSelectedPartitionCounts
call in ndv_test.go and the existing require.False assertion to replace with
count checks plus require.False.
In `@pkg/planner/core/plan_cache_partition_table_test.go`:
- Around line 364-365: The test unconditionally indexes show warnings result
which can panic if empty; change the two places that access tk.MustQuery(`show
warnings`).Rows()[0][2] to first capture warnings := tk.MustQuery(`show
warnings`).Rows(), assert they are present with require.NotEmpty(t, warnings,
"expected static partition prune warning") (or use len check like other tests),
then call requireStaticPartitionPruneWarning(t, warnings[0][2].(string)); update
references around the assertions involving
tk.Session().GetSessionVars().FoundInPlanCache and
requireStaticPartitionPruneWarning to use this defensive check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3041c2f8-7426-4b5c-85a1-62d0598c34f1
📒 Files selected for processing (9)
pkg/planner/cardinality/BUILD.bazelpkg/planner/cardinality/ndv_test.gopkg/planner/core/casetest/planstats/BUILD.bazelpkg/planner/core/casetest/planstats/plan_stats_test.gopkg/planner/core/logical_plan_builder.gopkg/planner/core/plan_cache.gopkg/planner/core/plan_cache_partition_table_test.gopkg/planner/core/rule_partition_processor.gopkg/planner/core/tests/prepare/BUILD.bazel
✅ Files skipped from review due to trivial changes (4)
- pkg/planner/core/casetest/planstats/BUILD.bazel
- pkg/planner/core/tests/prepare/BUILD.bazel
- pkg/planner/cardinality/BUILD.bazel
- pkg/planner/core/plan_cache.go
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/planner/core/casetest/planstats/plan_stats_test.go
- pkg/planner/core/rule_partition_processor.go
- pkg/planner/core/logical_plan_builder.go
Reminiscent
changed the title
Member
|
/retest |
hawkingrei
approved these changes
Mar 19, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: hawkingrei 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 |
[LGTM Timeline notifier]Timeline:
|
|
@Reminiscent: 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. |
|
@Reminiscent: 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. |
Contributor
|
@hawkingrei why did you approve this [DNM] PR of a backport of a non-merged PR? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
This is cherry pick of #66920
Issue Number: ref #66919
Problem Summary:
TiDB uses dynamic partition pruning by default. In some queries that only touch a subset of partitions, cardinality estimation still falls back to the partition table stats path, which is less accurate than using stats merged from the selected partitions. Reusing such partition-specific pruning information through plan cache is also unsafe.
What changed and how does it work?
Reuse statically-determined partition pruning results in the dynamic pruning path to collect the selected partition IDs for stats estimation.
Merge stats from the selected partitions at runtime instead of using partition table global stats.
Mark statements that rely on static partition pruning information as uncacheable, and skip cached plans when rebuilding them would depend on partition-specific pruning results.
Add regression coverage for merged partition stats and plan-cache behavior.
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
New Features
Improvements
Tests