Skip to content

planner: enable fix control 52869 by default to let planner consider IndexMerge when an alternative index exists | tidb-test=pr/2753#68920

Merged
ti-chi-bot[bot] merged 2 commits into
pingcap:release-8.5from
time-and-fate:s29-fix-control-52869-cp85
Jun 4, 2026
Merged

Conversation

@time-and-fate

@time-and-fate time-and-fate commented Jun 3, 2026

Copy link
Copy Markdown
Member

Manual cherry-pick of #63701


What problem does this PR solve?

Issue Number: ref #26764

Problem Summary:

Please see the issue and the doc.

We have been advising users to enable this flag for similar scenarios, and received no bad cases or bug reports. As we encounter this situation more and more often, we think it makes sense to enable this flag by default.

What changed and how does it work?

Set the default value to ON by default, and update the test results.

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.

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

  • Query Optimization
    • Adjusted index-merge and range-scan decision behavior; some queries may now produce different execution plans, changing which indexes drive lookups and predicate placement.
  • Tests
    • Updated expected query plan traces and cardinality-estimation debug outputs to reflect the new optimizer behavior.

@ti-chi-bot ti-chi-bot Bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/cherry-pick-not-approved size/M Denotes a PR that changes 30-99 lines, ignoring generated files. sig/planner SIG: Planner labels Jun 3, 2026
@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown

Review Change Stack

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: 9b584322-b2eb-45a3-a3dc-e7958885b9cd

📥 Commits

Reviewing files that changed from the base of the PR and between 59390fb and 0d470d3.

📒 Files selected for processing (2)
  • pkg/planner/cardinality/testdata/cardinality_suite_out.json
  • tests/integrationtest/r/imdbload.result
✅ Files skipped from review due to trivial changes (1)
  • tests/integrationtest/r/imdbload.result

📝 Walkthrough

Walkthrough

The PR changes Fix52869 default to true for index-merge range-scan validation and makes skylinePruning set preferMerge solely from explicit IndexMergeHints, causing planner selection and CE trace outputs to change across related tests.

Changes

IndexMerge Path Optimization Behavior

Layer / File(s) Summary
Optimizer feature flag default and path preference logic
pkg/planner/core/indexmerge_path.go, pkg/planner/core/find_best_task.go
generateIndexMerge4NormalIndex now treats fixcontrol.Fix52869 as true by default for skipRangeScanCheck; skylinePruning sets preferMerge based only on len(ds.IndexMergeHints) > 0.
Index access path plan expectations
tests/integrationtest/r/planner/core/indexmerge_path.result
Multiple EXPLAIN result expectations updated: equality+OR cases now show IndexLookUp with probe-side selections; several range/OR cases now show IndexMerge unions with secondary index range scans and adjusted TableRowIDScan estimates.
Cardinality/CE trace updates
pkg/planner/cardinality/testdata/cardinality_suite_out.json, tests/integrationtest/r/imdbload.result
CE trace JSON entries for OR-range predicates are split into multiple Index Stats-Range items with revised row_count values in both unit and integration test traces.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

size/S, cherry-pick-approved, ok-to-test, type/cherry-pick-for-release-8.5

Suggested reviewers

  • qw4990
  • terry1purcell
  • winoros

Poem

🐰 A tiny hop, a flag set true,
The planner trims the paths anew.
Hints now whisper which way to roam,
Traces split like crumbs back home.
Optimizer hums — the rabbit cheers!

🚥 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 accurately describes the main change: enabling fix control 52869 by default to allow planner to consider IndexMerge when alternative indexes exist.
Description check ✅ Passed The PR description follows the template structure and includes all required sections: issue reference, problem summary, what changed, checklist, side effects, and release notes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

@time-and-fate

Copy link
Copy Markdown
Member Author

/retest

@codecov

codecov Bot commented Jun 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (release-8.5@2fce0f9). Learn more about missing BASE report.

Additional details and impacted files
@@               Coverage Diff                @@
##             release-8.5     #68920   +/-   ##
================================================
  Coverage               ?   55.8546%           
================================================
  Files                  ?       1826           
  Lines                  ?     664246           
  Branches               ?          0           
================================================
  Hits                   ?     371012           
  Misses                 ?     266374           
  Partials               ?      26860           
Flag Coverage Δ
integration 39.2769% <100.0000%> (?)
unit 65.9364% <100.0000%> (?)

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

Components Coverage Δ
dumpling 55.3108% <0.0000%> (?)
parser ∅ <0.0000%> (?)
br 54.9383% <0.0000%> (?)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@time-and-fate time-and-fate changed the title planner: enable fix control 52869 by default to let planner consider IndexMerge when an alternative index exists planner: enable fix control 52869 by default to let planner consider IndexMerge when an alternative index exists | tidb-test=pr/2753 Jun 3, 2026
@time-and-fate

Copy link
Copy Markdown
Member Author

/retest

@ti-chi-bot ti-chi-bot Bot added cherry-pick-approved Cherry pick PR approved by release team. and removed do-not-merge/cherry-pick-not-approved labels Jun 4, 2026
@ti-chi-bot ti-chi-bot Bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Jun 4, 2026
@ti-chi-bot

ti-chi-bot Bot commented Jun 4, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

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

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 ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jun 4, 2026
@ti-chi-bot

ti-chi-bot Bot commented Jun 4, 2026

Copy link
Copy Markdown

[LGTM Timeline notifier]

Timeline:

  • 2026-06-04 14:40:30.780344609 +0000 UTC m=+452531.850661999: ☑️ agreed by qw4990.
  • 2026-06-04 15:37:37.529575824 +0000 UTC m=+455958.599893214: ☑️ agreed by terry1purcell.

@time-and-fate

Copy link
Copy Markdown
Member Author

/retest

@ti-chi-bot ti-chi-bot Bot merged commit 16cc308 into pingcap:release-8.5 Jun 4, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved cherry-pick-approved Cherry pick PR approved by release team. lgtm release-note-none Denotes a PR that doesn't merit a release note. sig/planner SIG: Planner size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants