pkg/planner: preserve suffix order for MergeJoin with constant leading keys

[qw4990]

What problem does this PR solve?

Issue Number: close #67755

Problem Summary: pkg/planner: preserve suffix order for MergeJoin with constant leading keys

What changed and how does it work?

• Make merge join sort-property matching treat leading join keys proven constant by FD inference as skippable.
• Add regression coverage for constant-left, constant-right, and non-constant control 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.

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

• Bug Fixes

  • Enhanced merge-join optimization to correctly handle queries with constant values in join conditions, enabling more efficient query plans.

• Tests

  • Added regression test for merge-join ordering scenarios.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ailinkid for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• pkg/planner/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds constant-column awareness to merge-join query planning. A new regression test validates the fix, while the core operator logic now recognizes that join keys proven constant via functional dependencies can be skipped during sort-order compatibility checking, enabling better query plans.

Changes

Cohort / File(s)	Summary
Regression Test pkg/planner/core/issuetest/planner_issue_test.go	New test case validating merge-join plan generation for multi-column primary key joins with constant filter predicates and required ordering.
Merge-Join Operator Logic pkg/planner/core/operator/physicalop/physical_merge_join.go	Extracts functional dependency constant columns and implements sort-property compatibility checking via new isSortPropCompatibleWithJoinKeys helper, allowing join keys proven constant to be skipped when validating order requirements. Updates outer-join rejection logic to use compatibility results instead of strict prefix checks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• #67588 — Modifies merge-join child-required-property logic in the same file, potentially conflicting or complementary with constant-column handling changes.

Suggested labels

contribution

Suggested reviewers

• hawkingrei
• terry1purcell
• AilinKid

Poem

🐰 A constant key walks through the join,
No longer forced to sort and toil,
The merge finds order, tried and true—
Skipping what it always knew! 🎯✨

🚥 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 summarizes the main change: making MergeJoin preserve suffix order when leading keys are constant.
Description check	✅ Passed	The description includes issue number, problem summary, and explanation of changes; however, the detailed 'What changed and how does it work?' section is quite brief and lacks technical depth.
Linked Issues check	✅ Passed	The PR directly addresses issue #67755 by implementing FD-aware constant-column skipping in MergeJoin sort-property matching and adds regression test coverage as specified.
Out of Scope Changes check	✅ Passed	All changes (planner logic and test) are directly scoped to fixing MergeJoin ordering with constant leading keys; no unrelated modifications detected.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[tiprow]

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

[qw4990]

/ok-to-test

[coderabbitai]

🧹 Nitpick comments (1)

pkg/planner/core/operator/physicalop/physical_merge_join.go (1)

539-571: Helper logic looks correct; consider documenting the uniform-direction precondition.

The scan is correct for the documented cases and correctly rejects when a non-constant, non-matching join key is encountered in the middle. A couple of small points worth noting:

• The function intentionally ignores SortItem.Desc and relies on the caller (tryToGetChildReqProp) having already enforced prop.AllSameOrder() and constructed lProp/rProp with the matching desc. This precondition is non-obvious from the signature alone; a brief note in the godoc would help future readers.
• A related (optional) generalization: a leading sort item whose column is itself in constantCols could also be skipped (order-by a fixed value is trivially satisfied). Not required here — the old IsPrefix didn't handle this either — but worth considering as a follow-up if similar patterns show up.

📝 Proposed doc tweak

 // isSortPropCompatibleWithJoinKeys checks whether the required order can be
 // satisfied by the join-key order when some leading join keys are fixed to a
 // single value.
+//
+// Precondition: all entries in sortItems share the same direction, and the
+// caller has built the child required properties with that same direction.
+// This function only checks column membership/order, not SortItem.Desc.
 //
 // Examples:

🤖 Prompt for AI Agents

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

In `@pkg/planner/core/operator/physicalop/physical_merge_join.go` around lines 539
- 571, Update the godoc for isSortPropCompatibleWithJoinKeys to state the
precondition that all SortItem directions are uniform (caller enforces
prop.AllSameOrder()) and that the caller (e.g., tryToGetChildReqProp) constructs
lProp/rProp with the matching Desc; explicitly note that this is why the
function ignores SortItem.Desc. Optionally mention that a future generalization
could skip leading sort items whose columns are in constantCols, but keep
current behavior unchanged.

🤖 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/core/operator/physicalop/physical_merge_join.go`:
- Around line 539-571: Update the godoc for isSortPropCompatibleWithJoinKeys to
state the precondition that all SortItem directions are uniform (caller enforces
prop.AllSameOrder()) and that the caller (e.g., tryToGetChildReqProp) constructs
lProp/rProp with the matching Desc; explicitly note that this is why the
function ignores SortItem.Desc. Optionally mention that a future generalization
could skip leading sort items whose columns are in constantCols, but keep
current behavior unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d3125383-03de-4b17-8803-074b1a0ebb94

📥 Commits

Reviewing files that changed from the base of the PR and between 8bde239 and 8b77226.

📒 Files selected for processing (2)

• pkg/planner/core/issuetest/planner_issue_test.go
• pkg/planner/core/operator/physicalop/physical_merge_join.go

[ti-chi-bot]

@qw4990: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
idc-jenkins-ci-tidb/build	8b77226	link	true	/test build
pull-build-next-gen	8b77226	link	true	/test pull-build-next-gen
idc-jenkins-ci-tidb/mysql-test	8b77226	link	true	/test mysql-test

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

[tiprow]

@qw4990: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
fast_test_tiprow	8b77226	link	true	/test fast_test_tiprow

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.1104%. Comparing base (2f4dff3) to head (8b77226).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #68001        +/-   ##
================================================
- Coverage   77.7969%   77.1104%   -0.6866%     
================================================
  Files          1984       1969        -15     
  Lines        549983     552199      +2216     
================================================
- Hits         427870     425803      -2067     
- Misses       121193     126185      +4992     
+ Partials        920        211       -709     

Flag	Coverage Δ
integration	40.8240% <100.0000%> (+1.0268%)	⬆️

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

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

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