Skip to content

planner: support pushing Limit and TopN to individual partial paths of IndexMerge (#68772) #68858

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ti-chi-bot wants to merge 1 commit into
base: release-8.5
Choose a base branch
from
Open

planner: support pushing Limit and TopN to individual partial paths of IndexMerge (#68772) #68858

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions pkg/executor/testdata/prepare_suite_out.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1208,9 +1208,15 @@
],
"Plan": [
"TopN_7 1.00 root test.t.b, offset:0, count:1",
<<<<<<< HEAD
"└─TableReader_14 1.00 root data:TopN_13",
" └─TopN_13 1.00 cop[tikv] test.t.b, offset:0, count:1",
" └─TableFullScan_12 10000.00 cop[tikv] table:t keep order:false, stats:pseudo"
=======
"└─TableReader_17 1.00 root data:TopN_16",
" └─TopN_16 1.00 cop[tikv] test.t.b, offset:0, count:1",
" └─TableFullScan_15 10000.00 cop[tikv] table:t keep order:false, stats:pseudo"
>>>>>>> 2310c3d99f3 (planner: support pushing Limit and TopN to individual partial paths of IndexMerge (#68772))
Comment on lines +1211 to +1219
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Resolve the leftover cherry-pick conflict markers.

These <<<<<<< / ======= / >>>>>>> markers make the fixture invalid JSON, so the prepare-suite testdata cannot even be parsed. Pick the intended plan variant and remove the conflict artifact before merge.

Also applies to: 1233-1241

🧰 Tools
🪛 Biome (2.4.16)

[error] 1211-1211: unexpected character <

(parse)

[error] 1211-1211: unexpected character <

(parse)

[error] 1211-1211: unexpected character <

(parse)

[error] 1211-1211: unexpected character <

(parse)

[error] 1211-1211: unexpected character <

(parse)

[error] 1211-1211: unexpected character <

(parse)

[error] 1211-1211: unexpected character <

(parse)

[error] 1211-1211: expected , but instead found HEAD

(parse)

[error] 1212-1212: expected , but instead found "└─TableReader_14 1.00 root data:TopN_13"

(parse)

[error] 1215-1215: unexpected character =

(parse)

[error] 1215-1215: unexpected character =

(parse)

[error] 1215-1215: unexpected character =

(parse)

[error] 1215-1215: unexpected character =

(parse)

[error] 1215-1215: unexpected character =

(parse)

[error] 1215-1215: unexpected character =

(parse)

[error] 1215-1215: unexpected character =

(parse)

[error] 1216-1216: expected , but instead found "└─TableReader_17 1.00 root data:TopN_16"

(parse)

[error] 1219-1219: unexpected character >

(parse)

[error] 1219-1219: unexpected character >

(parse)

[error] 1219-1219: unexpected character >

(parse)

[error] 1219-1219: unexpected character >

(parse)

[error] 1219-1219: unexpected character >

(parse)

[error] 1219-1219: unexpected character >

(parse)

[error] 1219-1219: unexpected character >

(parse)

[error] 1219-1219: expected , but instead found 2310

(parse)

[error] 1219-1219: expected , but instead found c3d99f3

(parse)

[error] 1219-1219: unexpected character (

(parse)

[error] 1219-1219: expected , but instead found planner

(parse)

[error] 1219-1219: expected , but instead found :

(parse)

[error] 1219-1219: unexpected character (

(parse)

[error] 1219-1219: unexpected character #

(parse)

[error] 1219-1219: expected , but instead found 68772

(parse)

[error] 1219-1219: unexpected character )

(parse)

[error] 1219-1219: unexpected character )

(parse)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/executor/testdata/prepare_suite_out.json` around lines 1211 - 1219, The
JSON fixture contains leftover git conflict markers (<<<<<<<, =======, >>>>>>>)
that break parsing; remove the conflict block and choose the correct plan lines
consistently (e.g., keep either the variant with
"TableReader_14/TopN_13/TableFullScan_12" or the variant with
"TableReader_17/TopN_16/TableFullScan_15") for the entries around the shown diff
and the similar block at lines 1233-1241 so the file becomes valid JSON with no
conflict markers.

],
"FromCache": "0"
},
Expand All @@ -1224,9 +1230,15 @@
],
"Plan": [
"TopN_7 5.00 root test.t.b, offset:0, count:5",
<<<<<<< HEAD
"└─TableReader_14 5.00 root data:TopN_13",
" └─TopN_13 5.00 cop[tikv] test.t.b, offset:0, count:5",
" └─TableFullScan_12 10000.00 cop[tikv] table:t keep order:false, stats:pseudo"
=======
"└─TableReader_17 5.00 root data:TopN_16",
" └─TopN_16 5.00 cop[tikv] test.t.b, offset:0, count:5",
" └─TableFullScan_15 10000.00 cop[tikv] table:t keep order:false, stats:pseudo"
>>>>>>> 2310c3d99f3 (planner: support pushing Limit and TopN to individual partial paths of IndexMerge (#68772))
],
"FromCache": "0"
},
Expand Down
3 changes: 2 additions & 1 deletion pkg/planner/cardinality/testdata/cardinality_suite_out.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1179,7 +1179,8 @@
"TopN 1.00 root test.t.c, offset:0, count:1",
"└─IndexMerge 1.00 root type: union",
" ├─IndexRangeScan(Build) 510.00 cop[tikv] table:t, index:ib(b) range:[0,50], keep order:false, stats:pseudo",
" ├─IndexRangeScan(Build) 510.00 cop[tikv] table:t, index:ic(c) range:[0,50], keep order:false, stats:pseudo",
" ├─Limit(Build) 1.00 cop[tikv] offset:0, count:1",
" │ └─IndexRangeScan 510.00 cop[tikv] table:t, index:ic(c) range:[0,50], keep order:true, stats:pseudo",
" └─TopN(Probe) 1.00 cop[tikv] test.t.c, offset:0, count:1",
" └─TableRowIDScan 1017.40 cop[tikv] table:t keep order:false, stats:pseudo"
Comment on lines 1179 to 1185
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

This expectation still misses the unordered partial-path TopN.

For the mixed-match IndexMerge case, the new behavior is supposed to push Limit to the ordered ic(c) branch and TopN to the unordered ib(b) branch. Here the ib(b) side is still a bare IndexRangeScan(Build), so this fixture does not actually lock in the selective pushdown behavior the PR is adding.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/planner/cardinality/testdata/cardinality_suite_out.json` around lines
1179 - 1185, The expectation is missing the unordered partial-path TopN that
should be pushed to the ib(b) branch; update the test fixture output so the
IndexMerge plan shows a TopN on the ib(b) side (i.e., the IndexRangeScan(Build)
for index:ib(b) should be wrapped with TopN(Probe) or equivalent unordered
TopN), while keeping the Limit(Build) on the ic(c) branch—adjust the lines
describing "IndexRangeScan(Build) table:t, index:ib(b) ..." to reflect the new
TopN pushdown behavior.

]
Expand Down
68 changes: 68 additions & 0 deletions pkg/planner/core/casetest/cbotest/testdata/analyze_suite_out.json
View file
Open in desktop

Large diffs are not rendered by default.

657 changes: 657 additions & 0 deletions pkg/planner/core/casetest/cbotest/testdata/analyze_suite_xut.json
View file
Open in desktop

Large diffs are not rendered by default.

12 changes: 12 additions & 0 deletions pkg/planner/core/casetest/testdata/integration_suite_out.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,30 @@
"SQL": "explain format = 'verbose' select * from t3 order by a limit 1",
"Plan": [
"TopN_7 1.00 60.74 root test.t3.a, offset:0, count:1",
<<<<<<< HEAD
"└─TableReader_15 1.00 54.34 root data:TopN_14",
" └─TopN_14 1.00 688.32 cop[tikv] test.t3.a, offset:0, count:1",
" └─TableFullScan_13 3.00 681.92 cop[tikv] table:t3 keep order:false"
=======
"└─TableReader_17 1.00 54.34 root data:TopN_16",
" └─TopN_16 1.00 688.32 cop[tikv] test.t3.a, offset:0, count:1",
" └─TableFullScan_15 3.00 681.92 cop[tikv] table:t3 keep order:false"
>>>>>>> 2310c3d99f3 (planner: support pushing Limit and TopN to individual partial paths of IndexMerge (#68772))
Comment on lines +70 to +78
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Resolve the leftover merge conflict markers.

These <<<<<<< / ======= / >>>>>>> markers make the fixture invalid JSON, so the casetest loader will fail before the updated plan expectations are exercised. Keep only the final expected plan block for each case.

As per coding guidelines, "Testdata: Keep test changes minimal and deterministic; avoid broad golden/testdata churn unless required; follow test naming and shard_count guidance".

Also applies to: 85-93

🧰 Tools
🪛 Biome (2.4.16)

[error] 70-70: unexpected character <

(parse)

[error] 70-70: unexpected character <

(parse)

[error] 70-70: unexpected character <

(parse)

[error] 70-70: unexpected character <

(parse)

[error] 70-70: unexpected character <

(parse)

[error] 70-70: unexpected character <

(parse)

[error] 70-70: unexpected character <

(parse)

[error] 70-70: expected , but instead found HEAD

(parse)

[error] 71-71: expected , but instead found "└─TableReader_15 1.00 54.34 root data:TopN_14"

(parse)

[error] 74-74: unexpected character =

(parse)

[error] 74-74: unexpected character =

(parse)

[error] 74-74: unexpected character =

(parse)

[error] 74-74: unexpected character =

(parse)

[error] 74-74: unexpected character =

(parse)

[error] 74-74: unexpected character =

(parse)

[error] 74-74: unexpected character =

(parse)

[error] 75-75: expected , but instead found "└─TableReader_17 1.00 54.34 root data:TopN_16"

(parse)

[error] 78-78: unexpected character >

(parse)

[error] 78-78: unexpected character >

(parse)

[error] 78-78: unexpected character >

(parse)

[error] 78-78: unexpected character >

(parse)

[error] 78-78: unexpected character >

(parse)

[error] 78-78: unexpected character >

(parse)

[error] 78-78: unexpected character >

(parse)

[error] 78-78: expected , but instead found 2310

(parse)

[error] 78-78: expected , but instead found c3d99f3

(parse)

[error] 78-78: unexpected character (

(parse)

[error] 78-78: expected , but instead found planner

(parse)

[error] 78-78: expected , but instead found :

(parse)

[error] 78-78: unexpected character (

(parse)

[error] 78-78: unexpected character #

(parse)

[error] 78-78: expected , but instead found 68772

(parse)

[error] 78-78: unexpected character )

(parse)

[error] 78-78: unexpected character )

(parse)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/planner/core/casetest/testdata/integration_suite_out.json` around lines
70 - 78, Remove the Git conflict markers and keep only the resolved expected
plan JSON for each conflicted block: remove the lines starting with <<<<<<<,
=======, and >>>>>>> and retain the final plan variant (e.g., the block
containing "└─TableReader_17", "└─TopN_16", "└─TableFullScan_15") so the JSON is
valid; apply the same fix to the second conflict region (the one around lines
showing "TableReader_15"/"TopN_14"/"TableFullScan_13") so both fixtures become
deterministic and parseable by the casetest loader.

]
},
{
"SQL": "explain format = 'verbose' select * from t3 order by b limit 1",
"Plan": [
"TopN_7 1.00 60.74 root test.t3.b, offset:0, count:1",
<<<<<<< HEAD
"└─TableReader_15 1.00 54.34 root data:TopN_14",
" └─TopN_14 1.00 688.32 cop[tikv] test.t3.b, offset:0, count:1",
" └─TableFullScan_13 3.00 681.92 cop[tikv] table:t3 keep order:false"
=======
"└─TableReader_17 1.00 54.34 root data:TopN_16",
" └─TopN_16 1.00 688.32 cop[tikv] test.t3.b, offset:0, count:1",
" └─TableFullScan_15 3.00 681.92 cop[tikv] table:t3 keep order:false"
>>>>>>> 2310c3d99f3 (planner: support pushing Limit and TopN to individual partial paths of IndexMerge (#68772))
]
},
{
Expand Down
1,380 changes: 1,380 additions & 0 deletions pkg/planner/core/casetest/testdata/integration_suite_xut.json
View file
Open in desktop

Large diffs are not rendered by default.

197 changes: 176 additions & 21 deletions pkg/planner/core/find_best_task.go
View file
Open in desktop

Large diffs are not rendered by default.

Loading