planner: refactor analyzeTiCIIndex for clearer structure | tidb-test=13ccf8de48e8db2290ff884598444d0508606bbf tiflash=feature-fts

[winoros]

What problem does this PR solve?

Issue Number: ref #66389

Problem Summary:
analyzeTiCIIndex in DataSource has dense, mixed responsibilities (dirty-write checks, TiCI path selection, and expression rewrite), making the control flow hard to follow and maintain.

What changed and how does it work?

Refactor analyzeTiCIIndex by moving logic into focused helpers while keeping behavior unchanged:

• Keep the top-level flow linear: dirty-write check -> choose index -> no-match cleanup -> rewrite matched FTS expressions -> build TiCI FTS path.
• Extract helper methods for candidate filtering, covered-column collection, matched-expression collection, and rewrite.
• Preserve the condition-level FTS detection semantics used for multi-table cases.

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.

Test command:
make failpoint-enable && (pushd pkg/planner/core/casetest/tici >/dev/null && go test -run 'TestTiCIWithDirtyWrites|TestTiCIMatchAgainstValidation' --tags=intest && popd >/dev/null; rc=$?; make failpoint-disable; exit $rc)

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

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Summary by CodeRabbit

• Refactor

  • Reworked FTS planning into a staged pipeline for more reliable index selection, expression rewriting, and path construction.

• Bug Fixes

  • Added pre-checks and earlier pruning of unsuitable index paths to avoid unsafe or incorrect index usage when FTS predicates or uncommitted writes are present.
  • Query planner now chooses different join/access strategies in some cases (e.g., HashJoin vs IndexJoin) to preserve correctness.

• Tests

  • Added integration tests covering FTS with joins and dirty-write scenarios.

[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: 6e155c72-4cff-435c-a9e9-0a97c42429b4

📥 Commits

Reviewing files that changed from the base of the PR and between 8211a36 and 3b24494.

📒 Files selected for processing (1)

• pkg/planner/core/casetest/tici/testdata/tici_index_suite_out.json

---

📝 Walkthrough

Walkthrough

analyzeTiCIIndex in logical_datasource.go is refactored into a staged, parameterized pipeline: new helper methods handle dirty-write checks, per-condition FTS detection, index candidacy/selection, expression rewriting, and path construction, with early-exit branches and TiCI-path pruning. (49 words)

Changes

Cohort / File(s)	Summary
TiCI Index Analysis Refactor pkg/planner/core/operator/logicalop/logical_datasource.go	Replaced monolithic analyzeTiCIIndex with a staged pipeline and added helpers: checkTiCIDirtyWrite, collectPushedDownCondsHasFTSFuncSet, chooseTiCIIndex, isTiCIIndexPathCandidate, collectTiCIIndexCoveredColumns, collectMatchedExprSetForTiCIIndex, removeTiCIIndexPathsFromPossibleAccessPaths, rewriteMatchedTiCIFTSExprs. Introduced early returns, FTS-aware index selection, rewriting, and path finalization.
TiCI Planner Tests pkg/planner/core/casetest/tici/tici_test.go	Added test scenarios in TestTiCIWithDirtyWrites exercising FTS predicates, dirty-write handling, and expected TiCI index usage (ensures idx_c is chosen and behavior under a dirty write).
Planner Test Output Update pkg/planner/core/casetest/tici/testdata/tici_index_suite_out.json	Updated expected execution plan for a test case: replaced an IndexJoin-based plan with a HashJoin plan and adjusted the subtree (IndexReader/IndexRangeScan and IndexLookUp shape changes).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• planner: fix the HasFTS check for multi table case (#66389) | tidb-test=13ccf8de48e8db2290ff884598444d0508606bbf tiflash=release-fts-202602 #66575: Directly related edits to TiCI/FTS planning and per-condition FTS handling in logical_datasource.go.

Suggested labels

size/XL

Suggested reviewers

• wshwsh12
• qw4990

Poem

🐰 I hopped through code with curious sight,
Carved a monolith into steps just right,
Checked for dirt and marked FTS spots,
Rewrote expressions, pruned some lots,
Now indexes hop home in clearer light. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main refactoring work on analyzeTiCIIndex for improved structure, though it includes test metadata tags that are not directly descriptive of the change itself.
Description check	✅ Passed	The description includes the required Issue Number (ref #66389), a clear Problem Summary explaining the refactoring rationale, a What Changed section outlining the approach, and a complete Check List with unit test marked and test command provided. All required template sections are addressed.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

📝 Coding Plan

• Generate coding plan for human review comments

---

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

[codecov]

Codecov Report

❌ Patch coverage is 92.17391% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.0204%. Comparing base (bd3b320) to head (3b24494).
⚠️ Report is 15 commits behind head on feature/fts.

Additional details and impacted files

@@                 Coverage Diff                 @@
##           feature/fts     #66583        +/-   ##
===================================================
- Coverage      76.8610%   75.0204%   -1.8406%     
===================================================
  Files             1960       1936        -24     
  Lines           555677     556268       +591     
===================================================
- Hits            427099     417315      -9784     
- Misses          127116     138947     +11831     
+ Partials          1462          6      -1456     

Flag	Coverage Δ
integration	39.6667% <29.5652%> (-5.9427%)	⬇️
unit	74.0925% <92.1739%> (+0.0444%)	⬆️

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

Components	Coverage Δ
dumpling	56.7044% <ø> (ø)
parser	∅ <ø> (∅)
br	48.7526% <ø> (-17.4921%)	⬇️

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pkg/planner/core/operator/logicalop/logical_datasource.go (1)

721-752: ⚠️ Potential issue | 🟠 Major

Don’t select a hinted TiCI path when no predicate matched.

Line 746 can still choose a forced index when tmpMatchedExprSet is empty. That path then reaches the TiCI build/cleanup flow with no matched conditions, after which fallback paths may already be pruned while the TiCI path still qualifies as “unused” because AccessConds stays empty. A USE_INDEX hint on a TiCI index that matches nothing can therefore strand the DataSource without a usable fallback.

🔧 Minimal guard

 		if !ds.collectMatchedExprSetForTiCIIndex(condHasFTSFunc, ftsCols, invertedIndexedCols, &tmpMatchedExprSet) {
 			continue
 		}
+		if tmpMatchedExprSet.IsEmpty() {
+			continue
+		}
 
 		// We get here means this index can cover all FTS functions.
 		hasUnmatchedFTSOverAllIdx = false

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/planner/core/operator/logicalop/logical_datasource.go` around lines 721 -
752, In chooseTiCIIndex, avoid selecting a forced (hinted) index when it matches
no predicates: change the selection condition around
matchedExprSetForChosenIndex so a path with path.Forced is only chosen if
tmpMatchedExprSet.Len() > 0; i.e. require non-empty tmpMatchedExprSet when
considering the (path.Forced && !matchedIndexIsHinted) branch while leaving the
tmpMatchedExprSet.Len() > matchedExprSetForChosenIndex.Len() branch unchanged;
update the logic in chooseTiCIIndex so forced hints cannot be picked when
tmpMatchedExprSet is empty to prevent stranded TiCI-only paths.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@pkg/planner/core/operator/logicalop/logical_datasource.go`:
- Around line 721-752: In chooseTiCIIndex, avoid selecting a forced (hinted)
index when it matches no predicates: change the selection condition around
matchedExprSetForChosenIndex so a path with path.Forced is only chosen if
tmpMatchedExprSet.Len() > 0; i.e. require non-empty tmpMatchedExprSet when
considering the (path.Forced && !matchedIndexIsHinted) branch while leaving the
tmpMatchedExprSet.Len() > matchedExprSetForChosenIndex.Len() branch unchanged;
update the logic in chooseTiCIIndex so forced hints cannot be picked when
tmpMatchedExprSet is empty to prevent stranded TiCI-only paths.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 26cec43f-00b6-49ff-96c8-0824d65dbae3

📥 Commits

Reviewing files that changed from the base of the PR and between 4694f0d and 8211a36.

📒 Files selected for processing (2)

• pkg/planner/core/casetest/tici/tici_test.go
• pkg/planner/core/operator/logicalop/logical_datasource.go

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: time-and-fate, wshwsh12

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 [time-and-fate]

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-03-12 19:52:12.617370655 +0000 UTC m=+552564.129428306: ☑️ agreed by time-and-fate.
• 2026-03-17 13:20:34.183204284 +0000 UTC m=+276761.270861821: ☑️ agreed by wshwsh12.

[winoros]

/retest

[winoros]

/retest

[winoros]

/retest