pkg/planner: remove redundant visited table state in stats usage collector

[pinkbit256]

What problem does this PR solve?

Issue Number: close #67912

Problem Summary:
collect_column_stats_usage maintained redundant visited-table states for plan replayer capture.

What changed and how does it work?

• Removed redundant collectVisitedTable and visitedtbls from the collector.
• Reused visitedPhysTblIDs for plan replayer table-stats recording.
• Clarified comments: visitedPhysTblIDs currently stores table meta IDs (DataSource.TableInfo.ID), not DataSource.PhysicalTableID.

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.

Unit tests:

• ./tools/check/failpoint-go-test.sh pkg/planner/core/rule -run 'TestSkipSystemTables|TestCollectPredicateColumns|TestCollectHistNeededColumns'
• ./tools/check/failpoint-go-test.sh pkg/planner/core -run TestPlanReplayerCaptureRecordJsonStats
• make lint

Side effects

• Performance regression: Consumes more CPU
• Performance regression: Consumes more Memory
• Breaking backward compatibility

Documentation

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features
• Changes MySQL compatibility

Release note

None

Summary by CodeRabbit

• Refactor
  • Simplified internal table-statistics tracking and plan-capture handling to reduce conditional state and streamline control flow.
  • Index-pruning column collection remains conditional; public interfaces and function signatures are unchanged.
  • Expected benefits: clearer code paths, easier maintenance, and potential reliability/performance improvements.

[pantheon-ai]

@pinkbit256 I've received your pull request and will start the review. I'll conduct a thorough review covering code quality, potential issues, and implementation details.

⏳ This process typically takes 10-30 minutes depending on the complexity of the changes.

_{ℹ️ Learn more details on Pantheon AI.}

[ti-chi-bot]

Hi @pinkbit256. 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]

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

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: dbe61ad4-2d3b-4ee3-89fc-b2c17447b09d

📥 Commits

Reviewing files that changed from the base of the PR and between 0248a67 and 92c02ae.

📒 Files selected for processing (1)

• pkg/planner/core/rule/collect_column_stats_usage.go

🚧 Files skipped from review as they are similar to previous changes (1)

• pkg/planner/core/rule/collect_column_stats_usage.go

---

📝 Walkthrough

Walkthrough

The collector now always records visited table logical IDs in visitedLogicalTblIDs; the constructor no longer accepts a plan-capture flag and CollectColumnStatsUsage performs plan-capture gating and converts visitedLogicalTblIDs into the visitedTbls map when needed.

Changes

Column Statistics Usage Collector Refactoring

Layer / File(s)	Summary
Data Structure pkg/planner/core/rule/collect_column_stats_usage.go	Replaces conditional plan-capture bookkeeping with an always-initialized visitedLogicalTblIDs set storing logical table IDs.
Constructor Changes pkg/planner/core/rule/collect_column_stats_usage.go	newColumnStatsUsageCollector drops the plan-capture parameter; always initializes visitedLogicalTblIDs; index-pruning fields (interestingColsByDS/colSet) remain conditional on collectIndexPruningCols.
Datasource Predicate Collection pkg/planner/core/rule/collect_column_stats_usage.go	During predicate collection, the current table's logical ID is inserted into visitedLogicalTblIDs unconditionally.
CollectColumnStatsUsage Orchestration pkg/planner/core/rule/collect_column_stats_usage.go	CollectColumnStatsUsage uses a local enablePlanCapture; when enabled it converts visitedLogicalTblIDs into visitedTbls and calls recordTableRuntimeStats.
Return Value / Docs pkg/planner/core/rule/collect_column_stats_usage.go	The function's second return value and its documentation now expose collector.visitedLogicalTblIDs (physical partition IDs remain in tblID2PartitionIDs).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰
I hopped through code with nimble paws,
Merged two tracks into one tidy cause,
Logical IDs now leap into sight,
Constructor lightened, the flow feels right,
A small refactor—clean, quick, and bright.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main refactoring: removal of redundant visited table state in the stats usage collector.
Description check	✅ Passed	The description covers the problem statement, what changed, testing approach, and includes proper issue link; all required sections are present and adequately filled.
Linked Issues check	✅ Passed	The PR successfully addresses all coding requirements from #67912: removes redundant visitedtbls, reuses visitedPhysTblIDs for plan replayer recording, and clarifies the field name to visitedLogicalTblIDs.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the stated objective of removing redundant visited table state; no unrelated modifications are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.1)

Command failed

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[hawkingrei]

/ok-to-test

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.8659%. Comparing base (511dba1) to head (92c02ae).
⚠️ Report is 31 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #68166        +/-   ##
================================================
- Coverage   77.7466%   76.8659%   -0.8808%     
================================================
  Files          1990       1973        -17     
  Lines        551792     555255      +3463     
================================================
- Hits         429000     426802      -2198     
- Misses       121872     128321      +6449     
+ Partials        920        132       -788     

Flag	Coverage Δ
integration	41.4347% <100.0000%> (+1.6329%)	⬆️

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

Components	Coverage Δ
dumpling	60.4888% <ø> (ø)
parser	∅ <ø> (∅)
br	50.0573% <ø> (-13.0338%)	⬇️

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

[0xPoe]

Thanks!

[0xPoe]

/retest

[0xPoe]

Thanks!

[Copilot]

Pull request overview

Refactors the column stats usage collector in the planner to remove redundant “visited table” state while preserving existing behavior for both stats-load logic and plan replayer capture.

Changes:

• Removed collectVisitedTable/visitedtbls and associated conditional tracking logic.
• Renamed visitedPhysTblIDs to visitedLogicalTblIDs to reflect that it stores DataSource.TableInfo.ID (logical/meta table ID), not DataSource.PhysicalTableID.
• Reused the FastIntSet of visited table IDs for plan replayer capture by materializing it into the map[int64]struct{} required by recordTableRuntimeStats.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 0xPoe, 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:

• pkg/planner/OWNERS [0xPoe,terry1purcell]

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]

[LGTM Timeline notifier]

Timeline:

• 2026-05-11 12:52:57.090843697 +0000 UTC m=+97345.623623016: ☑️ agreed by 0xPoe.
• 2026-05-11 22:37:19.203602003 +0000 UTC m=+132407.736381352: ☑️ agreed by terry1purcell.