planner: support pushing Limit and TopN to individual partial paths of IndexMerge (#68772)#68858
Conversation
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
ti-chi-bot
added
do-not-merge/hold
|
This cherry pick PR is for a release branch and has not yet been approved by triage owners. To merge this cherry pick:
DetailsInstructions 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. |
|
@time-and-fate This PR has conflicts, I have hold it. |
|
@ti-chi-bot: ## If you want to know how to resolve it, please read the guide in TiDB Dev Guide. DetailsInstructions 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 ti-community-infra/tichi repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
📝 WalkthroughWalkthroughThis PR implements support for pushing Limit and TopN operators into individual partial paths of IndexMerge plans. The work extends the physical property system with advisory sort items and partial-order metadata, introduces PhysicalTopN operator implementation, refactors IndexMerge candidate matching to compute per-partial match results, and updates task attachment logic to apply per-partial-path sort behavior. Test data and integration tests validate the new plan shapes and correctness. ChangesPush Limit/TopN into IndexMerge Partial Paths with Advisory Sort
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions 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. Comment |
|
@ti-chi-bot: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
@ti-chi-bot: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tests/integrationtest/r/planner/core/plan_cost_ver2.result (1)
8-32:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winThis expected-cost fixture still contains an unresolved cherry-pick conflict.
The conflict markers in this block leave two different plan/cost snapshots in the same
.resultfile, so the planner test cannot validate a single expected answer. Resolve the hunk and keep only the intended regenerated output.🤖 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 `@tests/integrationtest/r/planner/core/plan_cost_ver2.result` around lines 8 - 32, The file contains unresolved git conflict markers (<<<<<<<, =======, >>>>>>>) mixing two planner output variants (e.g., nodes TableReader_16 vs TableReader_19, TopN_15 vs TopN_18, Selection_14 vs Selection_17, TableFullScan_13 vs TableFullScan_16, TopN_8 differences); remove the conflict markers and the unwanted duplicate block and leave only the correct regenerated expected output for the test (i.e., a single contiguous plan snapshot consistent with current generator), ensuring no leftover markers remain so the test can validate a single expected answer.tests/integrationtest/r/executor/issues.result (1)
934-970:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winClean up the committed merge conflicts in these regression results.
Both
EXPLAIN ANALYZEsections still include Git conflict markers, which makes the checked-in expectation unusable and guarantees result drift in the integration suite. Resolve each hunk to one final output before regenerating or validating the file.Also applies to: 1006-1044
🤖 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 `@tests/integrationtest/r/executor/issues.result` around lines 934 - 970, The file contains unresolved Git conflict markers (<<<<<<<, =======, >>>>>>>) in the EXPLAIN ANALYZE blocks (e.g., around Limit_10/Limit_12, TableReader_17/TableReader_22, TableFullScan_15/TableFullScan_20 and the following Limit_11/Limit_12/Limit_13 groups); remove the conflict markers and choose the correct, single expected output for each hunk so the regression result is deterministic, then save the cleaned EXPLAIN ANALYZE output (apply the same cleanup to the later hunks noted in the comment, e.g., the section that includes Limit_11–Limit_13 and TableFullScan_17–TableFullScan_23) and regenerate/commit the file.tests/integrationtest/r/planner/core/indexmerge_path.result (1)
1215-1398:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Unresolved Git merge conflict markers present
This test result file contains unresolved Git merge conflict markers (
<<<<<<<,=======,>>>>>>>). The file must be manually resolved before merging:
- HEAD version (lines 1216-1223): Shows
estRowscolumn in explain output- Incoming version (lines 1225-1398): Different format without
estRowsplus extensive new test casesThe presence of these markers will cause:
- Test parsing failures
- Invalid golden file comparisons
- Build/CI failures
Action required: Manually resolve the conflict by choosing the correct output format and regenerating the expected results if needed.
🤖 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 `@tests/integrationtest/r/planner/core/indexmerge_path.result` around lines 1215 - 1398, The test result file contains unresolved git conflict markers (<<<<<<<, =======, >>>>>>>) around the explain output (notice the HEAD block with an extra "estRows" column vs the incoming block without it and the large new test cases including IndexMerge outputs and TopN/Limit cases); remove the conflict markers, decide which version to keep (merge fields as needed — e.g., keep the incoming expanded tests and drop the stray "estRows" header or reconcile headers across outputs), ensure the explain outputs (IndexMerge, IndexRangeScan, TableRowIDScan, TopN, Limit) are consistent, then regenerate the expected golden output (indexmerge_path.result) so CI uses the resolved file.
🧹 Nitpick comments (1)
pkg/planner/core/task.go (1)
1192-1195: ⚖️ Poor tradeoff
estimateMaxXForPartialOrderreturns constant 0, affecting row estimation.The TODO is noted, but returning 0 means
estimatedRows = partialOrderedLimit + 0, which may underestimate the actual rows processed. This affects cost calculations for the partial order optimization path.Consider adding a tracking issue for implementing this estimation, as it could impact query plan selection quality for partial order scenarios.
🤖 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/task.go` around lines 1192 - 1195, The function estimateMaxXForPartialOrder currently returns a constant 0 which underestimates rows used in partial-order planning; update estimateMaxXForPartialOrder in pkg/planner/core/task.go to compute and return a reasonable upper-bound (e.g., derive from TopN/bucket statistics and relevant session variables) so estimatedRows = partialOrderedLimit + estimateMaxXForPartialOrder() is accurate, and add a TODO comment plus create a tracking issue reference (issue ID or URL) in the comment to ensure full implementation and future improvements are tracked; ensure you reference the function name estimateMaxXForPartialOrder when committing the change.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@pkg/executor/testdata/prepare_suite_out.json`:
- Around line 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.
In `@pkg/planner/cardinality/testdata/cardinality_suite_out.json`:
- Around line 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.
In `@pkg/planner/core/casetest/cbotest/testdata/analyze_suite_out.json`:
- Around line 543-627: Resolve the Git conflict markers in
analyze_suite_out.json by removing the raw merge markers (<<<<<<<, =======,
>>>>>>>) and restore only the intended TestIssue62438 expectations instead of
the spliced test cases; specifically pick and keep the correct Plan block for
TestIssue62438 (the block that contains Projection_7 / Projection_21 /
IndexLookUp_20 or the updated Projection_32 / Projection_31 / IndexLookUp_30
according to the intended cherry-pick) and delete the unrelated
TestReproHashJoinIssue and subsequent test case fragments that were merged in
erroneously so the JSON is valid and only contains the expected TestIssue62438
entries.
In `@pkg/planner/core/casetest/testdata/integration_suite_out.json`:
- Around line 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.
In `@pkg/planner/core/find_best_task.go`:
- Around line 686-701: Resolve the leftover cherry-pick conflict by removing the
git conflict markers and keeping the updated IndexMerge fields introduced in the
new commit: retain partialOrderMatchResult (property.PartialOrderMatchResult),
matchWithAdvisorySortItems (bool), partialPathMatchResults
([]property.PhysicalPropMatchResult), isFullRange (bool), and eqOrInCount (int),
and ensure indexJoinCols remains present once; then update any helper
calls/imports that reference the old field names or the advisory-sort/task-base
wiring so usages of partialOrderMatchResult, matchWithAdvisorySortItems, and
partialPathMatchResults compile correctly (adjust imports for property package
if needed) and remove any duplicated declarations from the old implementation.
In `@pkg/planner/core/task.go`:
- Around line 1101-1104: Resolve the unresolved merge markers by removing the
conflict delimiters and merging the incoming helper functions into the file
alongside the existing method: keep the PhysicalExpand.Attach2Task method
signature and body from HEAD but incorporate the new helper functions
(handlePartialOrderTopN, estimateMaxXForPartialOrder,
handleAdvisorySortItemsForIndexMerge, attach2Task4PhysicalProjection,
attach2Task4PhysicalExpand) into the same file; ensure Attach2Task delegates to
or integrates with attach2Task4PhysicalExpand where appropriate so the new
advisory sort items and partial-order TopN logic are used, and verify there are
no duplicate symbols or leftover <<<<<<< / ======= / >>>>>>> markers.
- Around line 1054-1075: Remove the literal merge conflict markers and keep the
correct, consistent variant (use the incoming/updated form that references
physicalop.CopTask and the standalone function canPushDownToTiKV(p, copTask)
with exported fields like RootTaskConds, IdxMergePartPlans,
IdxMergePartPlansMatchResults, IndexPlanFinished, IdxMergeIsIntersection,
IdxMergeMatchWithAdvisorySortItems); ensure the surrounding block (the
IndexMerge advisory-sort handling and the call to
handleAdvisorySortItemsForIndexMerge(p, copTask)) is preserved, update imports
to include physicalop if needed, and delete the HEAD variant and the conflict
markers so the file compiles.
In `@pkg/planner/property/physical_property.go`:
- Around line 290-380: The file contains unresolved git conflict markers
(<<<<<<<, =======, >>>>>>>) that break compilation; remove these markers and
reconcile the two conflicting blocks so types and methods are defined once and
consistently — ensure PhysicalProperty struct fields (IndexJoinProp,
NoCopPushDown, PartialOrderInfo, AdvisorySortItems), the PartialOrderInfo type
and its AllSameOrder method, PartialOrderMatchResult, IndexJoinRuntimeProp and
its CloneEssentialFields method are present and correctly implemented (preserve
intended comments and field names), and apply the same cleanup for the other
reported ranges (around lines 487-575 and 596-603) so there are no leftover
conflict markers.
In `@tests/integrationtest/r/executor/index_lookup_pushdown_partition.result`:
- Around line 26-40: The .result file contains unresolved git conflict markers
with two variants of the expected plan (e.g., IndexLookUp_16 vs IndexLookUp_19,
LocalIndexLookUp_18 vs LocalIndexLookUp_21, TopN_15 vs TopN_18,
IndexRangeScan_13 vs IndexRangeScan_16); decide which regenerated plan is
correct for the test, remove the <<<<<<</=======/>>>>>>> scaffolding, keep only
the chosen hunk content (and repeat this for the other conflicted region at
lines 219-285), and ensure the file contains a single deterministic expected
plan.
In `@tests/integrationtest/r/executor/index_lookup_pushdown.result`:
- Around line 366-382: The test result file contains Git conflict markers
(<<<<<<<, =======, >>>>>>>) causing a conflicted artifact; open the failing
expected output block and remove the conflict markers, keeping only one
canonical plan tree (choose either the IndexLookUp_17/... block or the
IndexLookUp_20/... block) so the .result contains a single valid plan (ensure
you remove the marker lines and any duplicated plan copy left by the merge).
In `@tests/integrationtest/r/index_merge.result`:
- Around line 867-972: The file contains unresolved Git conflict markers
(<<<<<<< HEAD, =======, >>>>>>>) around the new test section; remove those
markers and accept the incoming change block (the entire "IndexMerge with IN
conditions and ORDER BY LIMIT..." section including table t_im_in creation,
inserts, analyze, the explain outputs referencing
IndexMerge_35/IndexRangeScan_30/31/TableRowIDScan_32, UnionScan_21, and the
PREPARE/EXECUTE example) so the result file contains only the incoming test
content (no conflict lines) and retains examples like the prepare stmt and
outputs as-is.
In `@tests/integrationtest/t/planner/core/indexmerge_path.test`:
- Around line 520-599: Resolve the unresolved git conflict markers by removing
the conflict delimiters (<<<<<<< HEAD, =======, >>>>>>> 2310c3d99f3) and keeping
the incoming test blocks under TestIssue58361 and TestIndexMergeSortItemsHints
(i.e., accept the incoming content for those tests), then update the
corresponding .result expectations to match the added tests and run the test
suite to verify parsing/execution succeed; locate the test sections by the
unique test headings "TestIssue58361" and "TestIndexMergeSortItemsHints" to
apply the fix.
---
Outside diff comments:
In `@tests/integrationtest/r/executor/issues.result`:
- Around line 934-970: The file contains unresolved Git conflict markers
(<<<<<<<, =======, >>>>>>>) in the EXPLAIN ANALYZE blocks (e.g., around
Limit_10/Limit_12, TableReader_17/TableReader_22,
TableFullScan_15/TableFullScan_20 and the following Limit_11/Limit_12/Limit_13
groups); remove the conflict markers and choose the correct, single expected
output for each hunk so the regression result is deterministic, then save the
cleaned EXPLAIN ANALYZE output (apply the same cleanup to the later hunks noted
in the comment, e.g., the section that includes Limit_11–Limit_13 and
TableFullScan_17–TableFullScan_23) and regenerate/commit the file.
In `@tests/integrationtest/r/planner/core/indexmerge_path.result`:
- Around line 1215-1398: The test result file contains unresolved git conflict
markers (<<<<<<<, =======, >>>>>>>) around the explain output (notice the HEAD
block with an extra "estRows" column vs the incoming block without it and the
large new test cases including IndexMerge outputs and TopN/Limit cases); remove
the conflict markers, decide which version to keep (merge fields as needed —
e.g., keep the incoming expanded tests and drop the stray "estRows" header or
reconcile headers across outputs), ensure the explain outputs (IndexMerge,
IndexRangeScan, TableRowIDScan, TopN, Limit) are consistent, then regenerate the
expected golden output (indexmerge_path.result) so CI uses the resolved file.
In `@tests/integrationtest/r/planner/core/plan_cost_ver2.result`:
- Around line 8-32: The file contains unresolved git conflict markers (<<<<<<<,
=======, >>>>>>>) mixing two planner output variants (e.g., nodes TableReader_16
vs TableReader_19, TopN_15 vs TopN_18, Selection_14 vs Selection_17,
TableFullScan_13 vs TableFullScan_16, TopN_8 differences); remove the conflict
markers and the unwanted duplicate block and leave only the correct regenerated
expected output for the test (i.e., a single contiguous plan snapshot consistent
with current generator), ensuring no leftover markers remain so the test can
validate a single expected answer.
---
Nitpick comments:
In `@pkg/planner/core/task.go`:
- Around line 1192-1195: The function estimateMaxXForPartialOrder currently
returns a constant 0 which underestimates rows used in partial-order planning;
update estimateMaxXForPartialOrder in pkg/planner/core/task.go to compute and
return a reasonable upper-bound (e.g., derive from TopN/bucket statistics and
relevant session variables) so estimatedRows = partialOrderedLimit +
estimateMaxXForPartialOrder() is accurate, and add a TODO comment plus create a
tracking issue reference (issue ID or URL) in the comment to ensure full
implementation and future improvements are tracked; ensure you reference the
function name estimateMaxXForPartialOrder when committing the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 60348d0e-30c2-43f3-9de6-e40abbd33cdc
📒 Files selected for processing (18)
pkg/executor/testdata/prepare_suite_out.jsonpkg/planner/cardinality/testdata/cardinality_suite_out.jsonpkg/planner/core/casetest/cbotest/testdata/analyze_suite_out.jsonpkg/planner/core/casetest/cbotest/testdata/analyze_suite_xut.jsonpkg/planner/core/casetest/testdata/integration_suite_out.jsonpkg/planner/core/casetest/testdata/integration_suite_xut.jsonpkg/planner/core/find_best_task.gopkg/planner/core/operator/physicalop/physical_topn.gopkg/planner/core/operator/physicalop/task_base.gopkg/planner/core/task.gopkg/planner/property/physical_property.gotests/integrationtest/r/executor/index_lookup_pushdown.resulttests/integrationtest/r/executor/index_lookup_pushdown_partition.resulttests/integrationtest/r/executor/issues.resulttests/integrationtest/r/index_merge.resulttests/integrationtest/r/planner/core/indexmerge_path.resulttests/integrationtest/r/planner/core/plan_cost_ver2.resulttests/integrationtest/t/planner/core/indexmerge_path.test
| <<<<<<< 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)) |
There was a problem hiding this comment.
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.
| "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" |
There was a problem hiding this comment.
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.
| <<<<<<< HEAD | ||
| "Projection_7 5001.00 9913298.19 ((((((((net(5001*rowsize(54.67)*tidb_kv_net_factor(3.96))) + (((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00)))/15.00) + (((((net(5001*rowsize(273.515)*tidb_kv_net_factor(3.96))) + ((scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00))/15.00) + ((double-read-cpu(5001*tidb_cpu_factor(49.9))) + (doubleRead(tasks(8.0016)*tidb_request_factor(6e+06)))))/5.00))*1.00)*1.00) + ((cpu(5001*filters(0.08)*tidb_cpu_factor(49.9)))/5.00)) + ((cpu(5001*filters(0.08)*tidb_cpu_factor(49.9)))/5.00)) + ((cpu(5001*filters(0.060000000000000005)*tidb_cpu_factor(49.9)))/5.00) root test.objects.path, test.objects.updated_ms, test.objects.size, test.objects.etag, test.objects.seq, test.objects.last_seen_ms", | ||
| "└─Projection_25 5001.00 9906310.79 ((((((net(5001*rowsize(54.67)*tidb_kv_net_factor(3.96))) + (((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00)))/15.00) + (((((net(5001*rowsize(273.515)*tidb_kv_net_factor(3.96))) + ((scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00))/15.00) + ((double-read-cpu(5001*tidb_cpu_factor(49.9))) + (doubleRead(tasks(8.0016)*tidb_request_factor(6e+06)))))/5.00))*1.00)*1.00) + ((cpu(5001*filters(0.08)*tidb_cpu_factor(49.9)))/5.00) root test.objects.path, test.objects.updated_ms, test.objects.size, test.objects.etag, test.objects.seq, test.objects.last_seen_ms, test.objects.metastore_uuid, test.objects.securable_id", | ||
| " └─IndexLookUp_24 5001.00 9902317.99 (((((net(5001*rowsize(54.67)*tidb_kv_net_factor(3.96))) + (((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00)))/15.00) + (((((net(5001*rowsize(273.515)*tidb_kv_net_factor(3.96))) + ((scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00))/15.00) + ((double-read-cpu(5001*tidb_cpu_factor(49.9))) + (doubleRead(tasks(8.0016)*tidb_request_factor(6e+06)))))/5.00))*1.00)*1.00 root limit embedded(offset:0, count:5001)", | ||
| " ├─Limit_23(Build) 5001.00 1253699.35 ((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00) cop[tikv] offset:0, count:5001", | ||
| " │ └─IndexRangeScan_21 5001.00 1253699.35 (scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00 cop[tikv] table:objects, index:idx_metastore_securable_seq(metastore_uuid, securable_id, seq) range:(\"<\\xbc\\xc2l\\xa7\\xe7@\\xa4\\x8d\\xd1d\\xd7GW\\xde\\xe2\" 2238365063123291 17299834,\"<\\xbc\\xc2l\\xa7\\xe7@\\xa4\\x8d\\xd1d\\xd7GW\\xde\\xe2\" 2238365063123291 +inf], keep order:true", | ||
| " └─TableRowIDScan_22(Probe) 5001.00 1688011.54 (scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00 cop[tikv] table:objects keep order:false" | ||
| ======= | ||
| "Projection_32 5001.00 9909305.39 (((((((net(5001*rowsize(54.67)*tidb_kv_net_factor(3.96))) + (((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00)))/15.00) + (((((net(5001*rowsize(273.515)*tidb_kv_net_factor(3.96))) + ((scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00))/15.00) + ((double-read-cpu(5001*tidb_cpu_factor(49.9))) + (doubleRead(tasks(8.0016)*tidb_request_factor(6e+06)))))/5.00))*1.00)*1.00) + ((cpu(5001*filters(0.08)*tidb_cpu_factor(49.9)))/5.00)) + ((cpu(5001*filters(0.060000000000000005)*tidb_cpu_factor(49.9)))/5.00) root test.objects.path, test.objects.updated_ms, test.objects.size, test.objects.etag, test.objects.seq, test.objects.last_seen_ms", | ||
| "└─Projection_31 5001.00 9906310.79 ((((((net(5001*rowsize(54.67)*tidb_kv_net_factor(3.96))) + (((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00)))/15.00) + (((((net(5001*rowsize(273.515)*tidb_kv_net_factor(3.96))) + ((scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00))/15.00) + ((double-read-cpu(5001*tidb_cpu_factor(49.9))) + (doubleRead(tasks(8.0016)*tidb_request_factor(6e+06)))))/5.00))*1.00)*1.00) + ((cpu(5001*filters(0.08)*tidb_cpu_factor(49.9)))/5.00) root test.objects.path, test.objects.updated_ms, test.objects.size, test.objects.etag, test.objects.seq, test.objects.last_seen_ms, test.objects.metastore_uuid, test.objects.securable_id", | ||
| " └─IndexLookUp_30 5001.00 9902317.99 (((((net(5001*rowsize(54.67)*tidb_kv_net_factor(3.96))) + (((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00)))/15.00) + (((((net(5001*rowsize(273.515)*tidb_kv_net_factor(3.96))) + ((scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00))/15.00) + ((double-read-cpu(5001*tidb_cpu_factor(49.9))) + (doubleRead(tasks(8.0016)*tidb_request_factor(6e+06)))))/5.00))*1.00)*1.00 root limit embedded(offset:0, count:5001)", | ||
| " ├─Limit_29(Build) 5001.00 1253699.35 ((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00) cop[tikv] offset:0, count:5001", | ||
| " │ └─IndexRangeScan_27 5001.00 1253699.35 (scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00 cop[tikv] table:objects, index:idx_metastore_securable_seq(metastore_uuid, securable_id, seq) range:(\"<\\xbc\\xc2l\\xa7\\xe7@\\xa4\\x8d\\xd1d\\xd7GW\\xde\\xe2\" 2238365063123291 17299834,\"<\\xbc\\xc2l\\xa7\\xe7@\\xa4\\x8d\\xd1d\\xd7GW\\xde\\xe2\" 2238365063123291 +inf], keep order:true", | ||
| " └─TableRowIDScan_28(Probe) 5001.00 1688011.54 (scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00 cop[tikv] table:objects keep order:false" | ||
| >>>>>>> 2310c3d99f3 (planner: support pushing Limit and TopN to individual partial paths of IndexMerge (#68772)) | ||
| ] | ||
| }, | ||
| { | ||
| "SQL": "explain format = 'cost_trace' \nSELECT\n /*+ limit_to_cop(), QB_NAME(`listObjectsWithPrefix_FilterByCreated_Since_Securable`) */\n path,\n updated_ms,\n size,\n etag,\n seq,\n last_seen_ms\nFROM\n objects FORCE INDEX(idx_metastore_securable_seq)\nWHERE\n metastore_uuid = 0x3CBCC26CA7E740A48DD164D74757DEE2\n AND securable_id = 2238365063123291\n AND (\n seq > 17299834\n AND TRUE\n )\nORDER BY\n seq\nLIMIT\n 5001;", | ||
| "Plan": [ | ||
| <<<<<<< HEAD | ||
| "Projection_7 5001.00 9913298.19 ((((((((net(5001*rowsize(54.67)*tidb_kv_net_factor(3.96))) + (((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00)))/15.00) + (((((net(5001*rowsize(273.515)*tidb_kv_net_factor(3.96))) + ((scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00))/15.00) + ((double-read-cpu(5001*tidb_cpu_factor(49.9))) + (doubleRead(tasks(8.0016)*tidb_request_factor(6e+06)))))/5.00))*1.00)*1.00) + ((cpu(5001*filters(0.08)*tidb_cpu_factor(49.9)))/5.00)) + ((cpu(5001*filters(0.08)*tidb_cpu_factor(49.9)))/5.00)) + ((cpu(5001*filters(0.060000000000000005)*tidb_cpu_factor(49.9)))/5.00) root test.objects.path, test.objects.updated_ms, test.objects.size, test.objects.etag, test.objects.seq, test.objects.last_seen_ms", | ||
| "└─Projection_21 5001.00 9906310.79 ((((((net(5001*rowsize(54.67)*tidb_kv_net_factor(3.96))) + (((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00)))/15.00) + (((((net(5001*rowsize(273.515)*tidb_kv_net_factor(3.96))) + ((scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00))/15.00) + ((double-read-cpu(5001*tidb_cpu_factor(49.9))) + (doubleRead(tasks(8.0016)*tidb_request_factor(6e+06)))))/5.00))*1.00)*1.00) + ((cpu(5001*filters(0.08)*tidb_cpu_factor(49.9)))/5.00) root test.objects.path, test.objects.updated_ms, test.objects.size, test.objects.etag, test.objects.seq, test.objects.last_seen_ms, test.objects.metastore_uuid, test.objects.securable_id", | ||
| " └─IndexLookUp_20 5001.00 9902317.99 (((((net(5001*rowsize(54.67)*tidb_kv_net_factor(3.96))) + (((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00)))/15.00) + (((((net(5001*rowsize(273.515)*tidb_kv_net_factor(3.96))) + ((scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00))/15.00) + ((double-read-cpu(5001*tidb_cpu_factor(49.9))) + (doubleRead(tasks(8.0016)*tidb_request_factor(6e+06)))))/5.00))*1.00)*1.00 root limit embedded(offset:0, count:5001)", | ||
| " ├─Limit_19(Build) 5001.00 1253699.35 ((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00) cop[tikv] offset:0, count:5001", | ||
| " │ └─IndexRangeScan_17 5001.00 1253699.35 (scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00 cop[tikv] table:objects, index:idx_metastore_securable_seq(metastore_uuid, securable_id, seq) range:(\"<\\xbc\\xc2l\\xa7\\xe7@\\xa4\\x8d\\xd1d\\xd7GW\\xde\\xe2\" 2238365063123291 17299834,\"<\\xbc\\xc2l\\xa7\\xe7@\\xa4\\x8d\\xd1d\\xd7GW\\xde\\xe2\" 2238365063123291 +inf], keep order:true", | ||
| " └─TableRowIDScan_18(Probe) 5001.00 1688011.54 (scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00 cop[tikv] table:objects keep order:false" | ||
| ======= | ||
| "Projection_32 5001.00 9909305.39 (((((((net(5001*rowsize(54.67)*tidb_kv_net_factor(3.96))) + (((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00)))/15.00) + (((((net(5001*rowsize(273.515)*tidb_kv_net_factor(3.96))) + ((scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00))/15.00) + ((double-read-cpu(5001*tidb_cpu_factor(49.9))) + (doubleRead(tasks(8.0016)*tidb_request_factor(6e+06)))))/5.00))*1.00)*1.00) + ((cpu(5001*filters(0.08)*tidb_cpu_factor(49.9)))/5.00)) + ((cpu(5001*filters(0.060000000000000005)*tidb_cpu_factor(49.9)))/5.00) root test.objects.path, test.objects.updated_ms, test.objects.size, test.objects.etag, test.objects.seq, test.objects.last_seen_ms", | ||
| "└─Projection_31 5001.00 9906310.79 ((((((net(5001*rowsize(54.67)*tidb_kv_net_factor(3.96))) + (((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00)))/15.00) + (((((net(5001*rowsize(273.515)*tidb_kv_net_factor(3.96))) + ((scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00))/15.00) + ((double-read-cpu(5001*tidb_cpu_factor(49.9))) + (doubleRead(tasks(8.0016)*tidb_request_factor(6e+06)))))/5.00))*1.00)*1.00) + ((cpu(5001*filters(0.08)*tidb_cpu_factor(49.9)))/5.00) root test.objects.path, test.objects.updated_ms, test.objects.size, test.objects.etag, test.objects.seq, test.objects.last_seen_ms, test.objects.metastore_uuid, test.objects.securable_id", | ||
| " └─IndexLookUp_30 5001.00 9902317.99 (((((net(5001*rowsize(54.67)*tidb_kv_net_factor(3.96))) + (((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00)))/15.00) + (((((net(5001*rowsize(273.515)*tidb_kv_net_factor(3.96))) + ((scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00))/15.00) + ((double-read-cpu(5001*tidb_cpu_factor(49.9))) + (doubleRead(tasks(8.0016)*tidb_request_factor(6e+06)))))/5.00))*1.00)*1.00 root limit embedded(offset:0, count:5001)", | ||
| " ├─Limit_29(Build) 5001.00 1253699.35 ((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00) cop[tikv] offset:0, count:5001", | ||
| " │ └─IndexRangeScan_27 5001.00 1253699.35 (scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00 cop[tikv] table:objects, index:idx_metastore_securable_seq(metastore_uuid, securable_id, seq) range:(\"<\\xbc\\xc2l\\xa7\\xe7@\\xa4\\x8d\\xd1d\\xd7GW\\xde\\xe2\" 2238365063123291 17299834,\"<\\xbc\\xc2l\\xa7\\xe7@\\xa4\\x8d\\xd1d\\xd7GW\\xde\\xe2\" 2238365063123291 +inf], keep order:true", | ||
| " └─TableRowIDScan_28(Probe) 5001.00 1688011.54 (scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00 cop[tikv] table:objects keep order:false" | ||
| ] | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| "Name": "TestReproHashJoinIssue", | ||
| "Cases": [ | ||
| { | ||
| "SQL": "explain format = 'brief' SELECT COUNT(*) FROM t_small s STRAIGHT_JOIN t_big b ON b.id1 = s.id4 WHERE s.id1 = '10001' AND s.id2 = 0 AND s.id3 = 123456789 AND b.id2 = 10001 AND b.id3 = 0 AND b.id4 = 20991231;", | ||
| "Ratio": 0, | ||
| "Plan": [ | ||
| "HashAgg 1.00 root funcs:count(1)->Column#16", | ||
| "└─IndexHashJoin 100000.00 root inner join, inner:IndexReader, outer key:repro_hash_join_issue.t_small.id4, inner key:repro_hash_join_issue.t_big.id1, equal cond:eq(repro_hash_join_issue.t_small.id4, repro_hash_join_issue.t_big.id1)", | ||
| " ├─TableReader(Build) 1000.00 root data:Selection", | ||
| " │ └─Selection 1000.00 cop[tikv] eq(repro_hash_join_issue.t_small.id1, \"10001\"), eq(repro_hash_join_issue.t_small.id2, 0), eq(repro_hash_join_issue.t_small.id3, 123456789)", | ||
| " │ └─TableFullScan 1000.00 cop[tikv] table:s keep order:false", | ||
| " └─IndexReader(Probe) 100000.00 root index:IndexRangeScan", | ||
| " └─IndexRangeScan 100000.00 cop[tikv] table:b, index:idx_id1_id2_id3_id4_id5(id1, id2, id3, id4, id5) range: decided by [eq(repro_hash_join_issue.t_big.id1, repro_hash_join_issue.t_small.id4) eq(repro_hash_join_issue.t_big.id2, 10001) eq(repro_hash_join_issue.t_big.id3, 0) eq(repro_hash_join_issue.t_big.id4, 20991231)], keep order:false" | ||
| ] | ||
| }, | ||
| { | ||
| "SQL": "explain format = 'brief' SELECT COUNT(*) FROM t_small s STRAIGHT_JOIN t_big b ON b.id1 = s.id4 WHERE s.id1 = '10001' AND s.id2 = 0 AND s.id3 = 123456789 AND b.id2 = 10001 AND b.id3 = 0 AND b.id4 = 20991231;", | ||
| "Ratio": 0.5, | ||
| "Plan": [ | ||
| "HashAgg 1.00 root funcs:count(1)->Column#16", | ||
| "└─HashJoin 100000.00 root inner join, equal:[eq(repro_hash_join_issue.t_small.id4, repro_hash_join_issue.t_big.id1)]", | ||
| " ├─TableReader(Build) 1000.00 root data:Selection", | ||
| " │ └─Selection 1000.00 cop[tikv] eq(repro_hash_join_issue.t_small.id1, \"10001\"), eq(repro_hash_join_issue.t_small.id2, 0), eq(repro_hash_join_issue.t_small.id3, 123456789)", | ||
| " │ └─TableFullScan 1000.00 cop[tikv] table:s keep order:false", | ||
| " └─IndexReader(Probe) 1000.00 root index:Selection", | ||
| " └─Selection 1000.00 cop[tikv] eq(repro_hash_join_issue.t_big.id2, 10001), eq(repro_hash_join_issue.t_big.id3, 0), eq(repro_hash_join_issue.t_big.id4, 20991231)", | ||
| " └─IndexFullScan 1000.00 cop[tikv] table:b, index:idx_id1_id2_id3_id4_id5(id1, id2, id3, id4, id5) keep order:false" | ||
| ] | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| "Name": "TestIndexJoinPreferIndexCoversMoreJoinKeyCols", | ||
| "Cases": [ | ||
| { | ||
| "SQL": "explain format ='brief' SELECT \n mp.col1,\n mp.col2,\n mp.col3,\n mp.col4,\n mp.col5,\n mp.col6\nFROM\n mp\n LEFT JOIN ab ON ab.col1 = mp.col2\n AND ab.col2 = mp.col6\nWHERE\n mp.col5 = 'PKR'\n AND mp.col3 = 1\n AND mp.col6 IN ('1685', '2239')\n AND mp.col4 >= 1764788400\n AND mp.col4 <= 1767466799\n AND ab.col3 IN ('cj9dkqw7', 'del_cj9dkqw7')\nORDER BY\n mp.col4 DESC\nLIMIT\n 100;", | ||
| "Plan": [ | ||
| "TopN 84.77 root test.mp.col4:desc, offset:0, count:100", | ||
| "└─IndexHashJoin 84.77 root inner join, inner:IndexLookUp, outer key:test.ab.col1, test.ab.col2, inner key:test.mp.col2, test.mp.col6, equal cond:eq(test.ab.col1, test.mp.col2), eq(test.ab.col2, test.mp.col6)", | ||
| " ├─Batch_Point_Get(Build) 4.00 root table:ab, index:idx_1(col3, col2) keep order:false, desc:false", | ||
| " └─IndexLookUp(Probe) 84.77 root ", | ||
| " ├─Selection(Build) 382.27 cop[tikv] in(test.mp.col6, \"1685\", \"2239\")", | ||
| " │ └─IndexRangeScan 382.27 cop[tikv] table:mp, index:idx_1(col2, col6, col7) range: decided by [eq(test.mp.col2, test.ab.col1) eq(test.mp.col6, test.ab.col2)], keep order:false", | ||
| " └─Selection(Probe) 84.77 cop[tikv] eq(test.mp.col3, 1), eq(test.mp.col5, \"PKR\"), ge(test.mp.col4, 1764788400), le(test.mp.col4, 1767466799)", | ||
| " └─TableRowIDScan 382.27 cop[tikv] table:mp keep order:false" | ||
| >>>>>>> 2310c3d99f3 (planner: support pushing Limit and TopN to individual partial paths of IndexMerge (#68772)) |
There was a problem hiding this comment.
Clean up the unresolved conflict block before this fixture can be used.
This hunk still contains raw merge markers, so analyze_suite_out.json is unparsable. It also appears to splice unrelated test cases into TestIssue62438, which would broaden this cherry-pick beyond the stated scope if the wrong side is kept. Resolve the conflict and keep only the intended TestIssue62438 expectations here.
🧰 Tools
🪛 Biome (2.4.16)
[error] 543-543: unexpected character <
(parse)
[error] 543-543: unexpected character <
(parse)
[error] 543-543: unexpected character <
(parse)
[error] 543-543: unexpected character <
(parse)
[error] 543-543: unexpected character <
(parse)
[error] 543-543: unexpected character <
(parse)
[error] 543-543: unexpected character <
(parse)
[error] 543-543: expected , but instead found HEAD
(parse)
[error] 544-544: expected , but instead found "Projection_7 5001.00 9913298.19 ((((((((net(5001*rowsize(54.67)*tidb_kv_net_factor(3.96))) + (((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00)))/15.00) + (((((net(5001*rowsize(273.515)*tidb_kv_net_factor(3.96))) + ((scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00))/15.00) + ((double-read-cpu(5001*tidb_cpu_factor(49.9))) + (doubleRead(tasks(8.0016)*tidb_request_factor(6e+06)))))/5.00))*1.00)*1.00) + ((cpu(5001*filters(0.08)*tidb_cpu_factor(49.9)))/5.00)) + ((cpu(5001*filters(0.08)*tidb_cpu_factor(49.9)))/5.00)) + ((cpu(5001*filters(0.060000000000000005)*tidb_cpu_factor(49.9)))/5.00) root test.objects.path, test.objects.updated_ms, test.objects.size, test.objects.etag, test.objects.seq, test.objects.last_seen_ms"
(parse)
[error] 550-550: unexpected character =
(parse)
[error] 550-550: unexpected character =
(parse)
[error] 550-550: unexpected character =
(parse)
[error] 550-550: unexpected character =
(parse)
[error] 550-550: unexpected character =
(parse)
[error] 550-550: unexpected character =
(parse)
[error] 550-550: unexpected character =
(parse)
[error] 551-551: expected , but instead found "Projection_32 5001.00 9909305.39 (((((((net(5001*rowsize(54.67)*tidb_kv_net_factor(3.96))) + (((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00)))/15.00) + (((((net(5001*rowsize(273.515)*tidb_kv_net_factor(3.96))) + ((scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00))/15.00) + ((double-read-cpu(5001*tidb_cpu_factor(49.9))) + (doubleRead(tasks(8.0016)*tidb_request_factor(6e+06)))))/5.00))*1.00)*1.00) + ((cpu(5001*filters(0.08)*tidb_cpu_factor(49.9)))/5.00)) + ((cpu(5001*filters(0.060000000000000005)*tidb_cpu_factor(49.9)))/5.00) root test.objects.path, test.objects.updated_ms, test.objects.size, test.objects.etag, test.objects.seq, test.objects.last_seen_ms"
(parse)
[error] 557-557: unexpected character >
(parse)
[error] 557-557: unexpected character >
(parse)
[error] 557-557: unexpected character >
(parse)
[error] 557-557: unexpected character >
(parse)
[error] 557-557: unexpected character >
(parse)
[error] 557-557: unexpected character >
(parse)
[error] 557-557: unexpected character >
(parse)
[error] 557-557: expected , but instead found 2310
(parse)
[error] 557-557: expected , but instead found c3d99f3
(parse)
[error] 557-557: unexpected character (
(parse)
[error] 557-557: expected , but instead found planner
(parse)
[error] 557-557: expected , but instead found :
(parse)
[error] 557-557: unexpected character (
(parse)
[error] 557-557: unexpected character #
(parse)
[error] 557-557: expected , but instead found 68772
(parse)
[error] 557-557: unexpected character )
(parse)
[error] 557-557: unexpected character )
(parse)
[error] 563-563: unexpected character <
(parse)
[error] 563-563: unexpected character <
(parse)
[error] 563-563: unexpected character <
(parse)
[error] 563-563: unexpected character <
(parse)
[error] 563-563: unexpected character <
(parse)
[error] 563-563: unexpected character <
(parse)
[error] 563-563: unexpected character <
(parse)
[error] 563-563: expected , but instead found HEAD
(parse)
[error] 564-564: expected , but instead found "Projection_7 5001.00 9913298.19 ((((((((net(5001*rowsize(54.67)*tidb_kv_net_factor(3.96))) + (((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00)))/15.00) + (((((net(5001*rowsize(273.515)*tidb_kv_net_factor(3.96))) + ((scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00))/15.00) + ((double-read-cpu(5001*tidb_cpu_factor(49.9))) + (doubleRead(tasks(8.0016)*tidb_request_factor(6e+06)))))/5.00))*1.00)*1.00) + ((cpu(5001*filters(0.08)*tidb_cpu_factor(49.9)))/5.00)) + ((cpu(5001*filters(0.08)*tidb_cpu_factor(49.9)))/5.00)) + ((cpu(5001*filters(0.060000000000000005)*tidb_cpu_factor(49.9)))/5.00) root test.objects.path, test.objects.updated_ms, test.objects.size, test.objects.etag, test.objects.seq, test.objects.last_seen_ms"
(parse)
[error] 570-570: unexpected character =
(parse)
[error] 570-570: unexpected character =
(parse)
[error] 570-570: unexpected character =
(parse)
[error] 570-570: unexpected character =
(parse)
[error] 570-570: unexpected character =
(parse)
[error] 570-570: unexpected character =
(parse)
[error] 570-570: unexpected character =
(parse)
[error] 571-571: expected , but instead found "Projection_32 5001.00 9909305.39 (((((((net(5001*rowsize(54.67)*tidb_kv_net_factor(3.96))) + (((scan(5001*logrowsize(71.47926389640116)*tikv_scan_factor(40.7)))*1.00)))/15.00) + (((((net(5001*rowsize(273.515)*tidb_kv_net_factor(3.96))) + ((scan(5001*logrowsize(313.69926389640113)*tikv_scan_factor(40.7)))*1.00))/15.00) + ((double-read-cpu(5001*tidb_cpu_factor(49.9))) + (doubleRead(tasks(8.0016)*tidb_request_factor(6e+06)))))/5.00))*1.00)*1.00) + ((cpu(5001*filters(0.08)*tidb_cpu_factor(49.9)))/5.00)) + ((cpu(5001*filters(0.060000000000000005)*tidb_cpu_factor(49.9)))/5.00) root test.objects.path, test.objects.updated_ms, test.objects.size, test.objects.etag, test.objects.seq, test.objects.last_seen_ms"
(parse)
[error] 627-627: unexpected character >
(parse)
[error] 627-627: unexpected character >
(parse)
[error] 627-627: unexpected character >
(parse)
[error] 627-627: unexpected character >
(parse)
[error] 627-627: unexpected character >
(parse)
[error] 627-627: unexpected character >
(parse)
[error] 627-627: unexpected character >
(parse)
[error] 627-627: expected , but instead found 2310
(parse)
[error] 627-627: expected , but instead found c3d99f3
(parse)
[error] 627-627: unexpected character (
(parse)
[error] 627-627: expected , but instead found planner
(parse)
[error] 627-627: expected , but instead found :
(parse)
[error] 627-627: unexpected character (
(parse)
[error] 627-627: unexpected character #
(parse)
[error] 627-627: expected , but instead found 68772
(parse)
[error] 627-627: unexpected character )
(parse)
[error] 627-627: unexpected character )
(parse)
🪛 OpenGrep (1.22.0)
[ERROR] 548-548: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 548-548: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 555-555: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 555-555: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 561-561: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 568-568: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 568-568: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 575-575: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 575-575: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
🤖 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/cbotest/testdata/analyze_suite_out.json` around
lines 543 - 627, Resolve the Git conflict markers in analyze_suite_out.json by
removing the raw merge markers (<<<<<<<, =======, >>>>>>>) and restore only the
intended TestIssue62438 expectations instead of the spliced test cases;
specifically pick and keep the correct Plan block for TestIssue62438 (the block
that contains Projection_7 / Projection_21 / IndexLookUp_20 or the updated
Projection_32 / Projection_31 / IndexLookUp_30 according to the intended
cherry-pick) and delete the unrelated TestReproHashJoinIssue and subsequent test
case fragments that were merged in erroneously so the JSON is valid and only
contains the expected TestIssue62438 entries.
| <<<<<<< 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)) |
There was a problem hiding this comment.
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.
| <<<<<<< HEAD | ||
| indexJoinCols int // how many index columns are used in access conditions in this IndexJoin. | ||
| ======= | ||
| // partialOrderMatch records the partial order match result for TopN optimization. | ||
| // When the matched is true, it means this path can provide partial order using prefix index. | ||
| partialOrderMatchResult property.PartialOrderMatchResult // Result of matching partial order property | ||
| // matchWithAdvisorySortItems indicates the property matching used SortItemsHints | ||
| // (i.e. noSortItem && len(prop.SortItemsHints) > 0). Only relevant for IndexMerge. | ||
| matchWithAdvisorySortItems bool | ||
| // partialPathMatchResults stores each partial path's matchProperty result. | ||
| // Length equals len(path.PartialIndexPaths). Only set for IndexMerge paths. | ||
| partialPathMatchResults []property.PhysicalPropMatchResult | ||
| indexJoinCols int // how many index columns are used in access conditions in this IndexJoin. | ||
| isFullRange bool // cached result of whether this path covers the full scan range. | ||
| eqOrInCount int // cached result of equalPredicateCount(). | ||
| >>>>>>> 2310c3d99f3 (planner: support pushing Limit and TopN to individual partial paths of IndexMerge (#68772)) |
There was a problem hiding this comment.
Finish resolving the cherry-pick in this file.
The remaining conflict blocks interleave the old IndexMerge implementation with the new advisory-sort/task-base wiring, so this file will not build as-is and the selected branch still needs its helper/import/field-name updates reconciled.
Also applies to: 1139-1195, 1231-1236, 1323-1332, 1374-1385, 1391-1412, 2014-2024, 2050-2054, 2087-2096
🤖 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/find_best_task.go` around lines 686 - 701, Resolve the
leftover cherry-pick conflict by removing the git conflict markers and keeping
the updated IndexMerge fields introduced in the new commit: retain
partialOrderMatchResult (property.PartialOrderMatchResult),
matchWithAdvisorySortItems (bool), partialPathMatchResults
([]property.PhysicalPropMatchResult), isFullRange (bool), and eqOrInCount (int),
and ensure indexJoinCols remains present once; then update any helper
calls/imports that reference the old field names or the advisory-sort/task-base
wiring so usages of partialOrderMatchResult, matchWithAdvisorySortItems, and
partialPathMatchResults compile correctly (adjust imports for property package
if needed) and remove any duplicated declarations from the old implementation.
| <<<<<<< HEAD | ||
| ======= | ||
|
|
||
| IndexJoinProp *IndexJoinRuntimeProp | ||
|
|
||
| // NoCopPushDown indicates if planner must not push this agg down to coprocessor. | ||
| // It is true when the agg is in the outer child tree of apply. | ||
| NoCopPushDown bool | ||
|
|
||
| // PartialOrderInfo is used for TopN's partial order optimization. | ||
| // When this field is not nil, it indicates that prefix index can be used | ||
| // to provide partial order for TopN. | ||
| // For example: | ||
| // query: order by a, b limit 10 | ||
| // partialOrderInfo: sortItems: [a, b] | ||
| // The partialOrderInfo property will pass through to the datasource and try to matchPartialOrderProperty such as: | ||
| // index: (a, b(10) ) | ||
| PartialOrderInfo *PartialOrderInfo | ||
|
|
||
| // AdvisorySortItems contains sort items that are preferred but not required. | ||
| // When SortItems is empty and AdvisorySortItems is not, DataSource can try to | ||
| // generate paths that satisfy these sort items, enabling Limit pushdown to | ||
| // partial paths of IndexMerge. | ||
| // Currently only set when TopN is directly above a DataSource. | ||
| AdvisorySortItems []SortItem | ||
| } | ||
|
|
||
| // PartialOrderInfo records information needed for partial order optimization. | ||
| // When PhysicalProperty.PartialOrderInfo is not nil, it indicates that | ||
| // prefix index can be used to provide partial order. | ||
| type PartialOrderInfo struct { | ||
| // SortItems are the ORDER BY columns from TopN | ||
| SortItems []*SortItem | ||
| } | ||
|
|
||
| // AllSameOrder checks if all the items have same order. | ||
| func (p *PartialOrderInfo) AllSameOrder() (isSame bool, desc bool) { | ||
| if len(p.SortItems) == 0 { | ||
| return true, false | ||
| } | ||
| for i := 1; i < len(p.SortItems); i++ { | ||
| if p.SortItems[i].Desc != p.SortItems[i-1].Desc { | ||
| return | ||
| } | ||
| } | ||
| return true, p.SortItems[0].Desc | ||
| } | ||
|
|
||
| // PartialOrderMatchResult records the result of matching partial order property with an access path. | ||
| // It is stored in candidatePath to allow each path to have its own match result. | ||
| type PartialOrderMatchResult struct { | ||
| // Matched indicates whether this path can provide partial order | ||
| Matched bool | ||
| // PrefixCol is the last and only one prefix column ID of index, only used for executor part | ||
| // For example: | ||
| // Query ORDER BY a,b,c | ||
| // Index: a, b, c(10) | ||
| // PrefixCol: c, the col c | ||
| // PrefixLen: 10, the col length of c in index | ||
| PrefixCol *expression.Column | ||
|
|
||
| // PrefixLen is the prefix length in bytes for prefix index, only used for executor part | ||
| PrefixLen int | ||
| } | ||
|
|
||
| // IndexJoinRuntimeProp is the inner runtime property for index join. | ||
| type IndexJoinRuntimeProp struct { | ||
| // for complete the last col range access, cuz its runtime constant. | ||
| OtherConditions []expression.Expression | ||
| // for filling the range msg info | ||
| OuterJoinKeys []*expression.Column | ||
| // for inner ds/index to detect the range, cuz its runtime constant. | ||
| InnerJoinKeys []*expression.Column | ||
| // AvgInnerRowCnt is computed from join.EqualCondCount / outerChild.RowCount. | ||
| // since ds only can build empty range before seeing runtime data, the so inner | ||
| // ds can get an accurate countAfterAccess. Once index join prop pushed to the | ||
| // deeper side like through join, the deeper DS's countAfterAccess should be | ||
| // thought twice. | ||
| AvgInnerRowCnt float64 | ||
| // since tableRangeScan and indexRangeScan can't be told which one is better at | ||
| // copTask phase because of the latter attached operators into cop and the single | ||
| // and double reader cost consideration. Therefore, we introduce another bool to | ||
| // indicate prefer tableRangeScan or indexRangeScan each at a time. | ||
| TableRangeScan bool | ||
| } | ||
|
|
||
| // CloneEssentialFields clone the essential fields for IndexJoinRuntimeProp. | ||
| func (ijr *IndexJoinRuntimeProp) CloneEssentialFields() *IndexJoinRuntimeProp { | ||
| one := *ijr | ||
| return &one | ||
| >>>>>>> 2310c3d99f3 (planner: support pushing Limit and TopN to individual partial paths of IndexMerge (#68772)) |
There was a problem hiding this comment.
Resolve the leftover merge conflicts before merging.
This file still contains <<<<<<< / ======= / >>>>>>> blocks, so PhysicalProperty and its hash/clone paths cannot compile in the current state.
Also applies to: 487-575, 596-603
🤖 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/property/physical_property.go` around lines 290 - 380, The file
contains unresolved git conflict markers (<<<<<<<, =======, >>>>>>>) that break
compilation; remove these markers and reconcile the two conflicting blocks so
types and methods are defined once and consistently — ensure PhysicalProperty
struct fields (IndexJoinProp, NoCopPushDown, PartialOrderInfo,
AdvisorySortItems), the PartialOrderInfo type and its AllSameOrder method,
PartialOrderMatchResult, IndexJoinRuntimeProp and its CloneEssentialFields
method are present and correctly implemented (preserve intended comments and
field names), and apply the same cleanup for the other reported ranges (around
lines 487-575 and 596-603) so there are no leftover conflict markers.
| <<<<<<< HEAD | ||
| └─IndexLookUp_16 3.00 root partition:all | ||
| ├─LocalIndexLookUp_18(Build) 3.00 cop[tikv] index handle offsets:[1] | ||
| │ ├─TopN_15(Build) 3.00 cop[tikv] executor__index_lookup_pushdown_partition.tp1.a, offset:0, count:3 | ||
| │ │ └─IndexRangeScan_13 3323.33 cop[tikv] table:tp1, index:b(b) range:[-inf,10), keep order:false, stats:pseudo | ||
| │ └─TableRowIDScan_17(Probe) 3.00 cop[tikv] table:tp1 keep order:false, stats:pseudo | ||
| └─TableRowIDScan_14(Probe) 0.00 cop[tikv] table:tp1 keep order:false, stats:pseudo | ||
| ======= | ||
| └─IndexLookUp_19 3.00 root partition:all | ||
| ├─LocalIndexLookUp_21(Build) 3.00 cop[tikv] index handle offsets:[1] | ||
| │ ├─TopN_18(Build) 3.00 cop[tikv] executor__index_lookup_pushdown_partition.tp1.a, offset:0, count:3 | ||
| │ │ └─IndexRangeScan_16 3323.33 cop[tikv] table:tp1, index:b(b) range:[-inf,10), keep order:false, stats:pseudo | ||
| │ └─TableRowIDScan_20(Probe) 3.00 cop[tikv] table:tp1 keep order:false, stats:pseudo | ||
| └─TableRowIDScan_17(Probe) 0.00 cop[tikv] table:tp1 keep order:false, stats:pseudo | ||
| >>>>>>> 2310c3d99f3 (planner: support pushing Limit and TopN to individual partial paths of IndexMerge (#68772)) |
There was a problem hiding this comment.
Resolve both conflicted expected-plan hunks before merging.
These conflict markers leave two incompatible result variants in the same .result file, so the integration test cannot verify a deterministic output. Pick the intended regenerated plan for each hunk and remove the conflict scaffolding entirely.
Also applies to: 219-285
🤖 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 `@tests/integrationtest/r/executor/index_lookup_pushdown_partition.result`
around lines 26 - 40, The .result file contains unresolved git conflict markers
with two variants of the expected plan (e.g., IndexLookUp_16 vs IndexLookUp_19,
LocalIndexLookUp_18 vs LocalIndexLookUp_21, TopN_15 vs TopN_18,
IndexRangeScan_13 vs IndexRangeScan_16); decide which regenerated plan is
correct for the test, remove the <<<<<<</=======/>>>>>>> scaffolding, keep only
the chosen hunk content (and repeat this for the other conflicted region at
lines 219-285), and ensure the file contains a single deterministic expected
plan.
| <<<<<<< HEAD | ||
| └─IndexLookUp_17 4.00 root | ||
| ├─LocalIndexLookUp_19(Build) 4.00 cop[tikv] index handle offsets:[] | ||
| │ ├─TopN_16(Build) 4.00 cop[tikv] executor__index_lookup_pushdown.t2.a:desc, executor__index_lookup_pushdown.t2.b:desc, offset:0, count:4 | ||
| │ │ └─Selection_15 6656.67 cop[tikv] ne(executor__index_lookup_pushdown.t2.b, 3) | ||
| │ │ └─IndexFullScan_13 10000.00 cop[tikv] table:t2, index:i(c) keep order:false, stats:pseudo | ||
| │ └─TableRowIDScan_18(Probe) 4.00 cop[tikv] table:t2 keep order:false, stats:pseudo | ||
| └─TableRowIDScan_14(Probe) 0.00 cop[tikv] table:t2 keep order:false, stats:pseudo | ||
| ======= | ||
| └─IndexLookUp_20 4.00 root | ||
| ├─LocalIndexLookUp_22(Build) 4.00 cop[tikv] index handle offsets:[] | ||
| │ ├─TopN_19(Build) 4.00 cop[tikv] executor__index_lookup_pushdown.t2.a:desc, executor__index_lookup_pushdown.t2.b:desc, offset:0, count:4 | ||
| │ │ └─Selection_18 6656.67 cop[tikv] ne(executor__index_lookup_pushdown.t2.b, 3) | ||
| │ │ └─IndexFullScan_16 10000.00 cop[tikv] table:t2, index:i(c) keep order:false, stats:pseudo | ||
| │ └─TableRowIDScan_21(Probe) 4.00 cop[tikv] table:t2 keep order:false, stats:pseudo | ||
| └─TableRowIDScan_17(Probe) 0.00 cop[tikv] table:t2 keep order:false, stats:pseudo | ||
| >>>>>>> 2310c3d99f3 (planner: support pushing Limit and TopN to individual partial paths of IndexMerge (#68772)) |
There was a problem hiding this comment.
Remove the unresolved conflict block from this expected result.
The <<<<<<< / ======= / >>>>>>> lines are still in the checked-in .result file, so the integration test now asserts against a conflicted artifact instead of a single canonical plan. Resolve the hunk and keep only one expected plan tree.
🤖 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 `@tests/integrationtest/r/executor/index_lookup_pushdown.result` around lines
366 - 382, The test result file contains Git conflict markers (<<<<<<<, =======,
>>>>>>>) causing a conflicted artifact; open the failing expected output block
and remove the conflict markers, keeping only one canonical plan tree (choose
either the IndexLookUp_17/... block or the IndexLookUp_20/... block) so the
.result contains a single valid plan (ensure you remove the marker lines and any
duplicated plan copy left by the merge).
| <<<<<<< HEAD | ||
| ======= | ||
| ///// IndexMerge with IN conditions and ORDER BY LIMIT (merge sort for partial paths) | ||
| drop table if exists t_im_in; | ||
| create table t_im_in ( | ||
| id int not null, | ||
| a int default null, | ||
| b int default null, | ||
| c int default null, | ||
| padding varchar(100) default null, | ||
| primary key (id) clustered, | ||
| key idx_a_c (a, c), | ||
| key idx_b_c (b, c) | ||
| ); | ||
| insert into t_im_in values | ||
| (1, 1, 1, 10, 'row1'), (2, 1, 2, 20, 'row2'), (3, 2, 1, 30, 'row3'), | ||
| (4, 2, 2, 40, 'row4'), (5, 1, 3, 50, 'row5'), (6, 3, 1, 60, 'row6'), | ||
| (7, 3, 2, 70, 'row7'), (8, 1, 1, 5, 'row8'), (9, 2, 3, 15, 'row9'), | ||
| (10, 3, 3, 25, 'row10'); | ||
| analyze table t_im_in; | ||
| # Case 1: a=1 keeps order directly, b IN (1,2) needs merge sort | ||
| explain select /*+ use_index_merge(t_im_in) */ * from t_im_in where a = 1 or b in (1, 2) order by c limit 5; | ||
| id estRows task access object operator info | ||
| IndexMerge_35 5.00 root type: union, limit embedded(offset:0, count:5) | ||
| ├─Limit_33(Build) 2.44 cop[tikv] offset:0, count:5 | ||
| │ └─IndexRangeScan_30 2.44 cop[tikv] table:t_im_in, index:idx_a_c(a, c) range:[1,1], keep order:true | ||
| ├─Limit_34(Build) 4.27 cop[tikv] offset:0, count:5 | ||
| │ └─IndexRangeScan_31 4.27 cop[tikv] table:t_im_in, index:idx_b_c(b, c) range:[1,1], [2,2], keep order:true | ||
| └─TableRowIDScan_32(Probe) 5.00 cop[tikv] table:t_im_in keep order:false | ||
| select /*+ use_index_merge(t_im_in) */ * from t_im_in where a = 1 or b in (1, 2) order by c limit 5; | ||
| id a b c padding | ||
| 8 1 1 5 row8 | ||
| 1 1 1 10 row1 | ||
| 2 1 2 20 row2 | ||
| 3 2 1 30 row3 | ||
| 4 2 2 40 row4 | ||
| # Case 2: Both partial paths need merge sort | ||
| explain select /*+ use_index_merge(t_im_in) */ * from t_im_in where a in (1, 2) or b in (1, 2) order by c limit 5; | ||
| id estRows task access object operator info | ||
| IndexMerge_35 5.00 root type: union, limit embedded(offset:0, count:5) | ||
| ├─Limit_33(Build) 3.85 cop[tikv] offset:0, count:5 | ||
| │ └─IndexRangeScan_30 3.85 cop[tikv] table:t_im_in, index:idx_a_c(a, c) range:[1,1], [2,2], keep order:true | ||
| ├─Limit_34(Build) 3.85 cop[tikv] offset:0, count:5 | ||
| │ └─IndexRangeScan_31 3.85 cop[tikv] table:t_im_in, index:idx_b_c(b, c) range:[1,1], [2,2], keep order:true | ||
| └─TableRowIDScan_32(Probe) 5.00 cop[tikv] table:t_im_in keep order:false | ||
| select /*+ use_index_merge(t_im_in) */ * from t_im_in where a in (1, 2) or b in (1, 2) order by c limit 5; | ||
| id a b c padding | ||
| 8 1 1 5 row8 | ||
| 1 1 1 10 row1 | ||
| 9 2 3 15 row9 | ||
| 2 1 2 20 row2 | ||
| 3 2 1 30 row3 | ||
| # Case 3: DESC ordering with IN condition | ||
| explain select /*+ use_index_merge(t_im_in) */ * from t_im_in where a = 1 or b in (1, 2) order by c desc limit 5; | ||
| id estRows task access object operator info | ||
| IndexMerge_35 5.00 root type: union, limit embedded(offset:0, count:5) | ||
| ├─Limit_33(Build) 2.44 cop[tikv] offset:0, count:5 | ||
| │ └─IndexRangeScan_30 2.44 cop[tikv] table:t_im_in, index:idx_a_c(a, c) range:[1,1], keep order:true, desc | ||
| ├─Limit_34(Build) 4.27 cop[tikv] offset:0, count:5 | ||
| │ └─IndexRangeScan_31 4.27 cop[tikv] table:t_im_in, index:idx_b_c(b, c) range:[1,1], [2,2], keep order:true, desc | ||
| └─TableRowIDScan_32(Probe) 5.00 cop[tikv] table:t_im_in keep order:false | ||
| select /*+ use_index_merge(t_im_in) */ * from t_im_in where a = 1 or b in (1, 2) order by c desc limit 5; | ||
| id a b c padding | ||
| 7 3 2 70 row7 | ||
| 6 3 1 60 row6 | ||
| 5 1 3 50 row5 | ||
| 4 2 2 40 row4 | ||
| 3 2 1 30 row3 | ||
| # Case 4: UnionScan - verify correctness with uncommitted data | ||
| begin; | ||
| insert into t_im_in values (11, 1, 1, 1, 'uncommitted'); | ||
| explain select /*+ use_index_merge(t_im_in) */ * from t_im_in where a = 1 or b in (1, 2) order by c limit 5; | ||
| id estRows task access object operator info | ||
| Limit_15 5.00 root offset:0, count:5 | ||
| └─UnionScan_21 5.00 root or(eq(index_merge.t_im_in.a, 1), in(index_merge.t_im_in.b, 1, 2)) | ||
| └─IndexMerge_25 5.00 root type: union | ||
| ├─IndexRangeScan_22(Build) 2.44 cop[tikv] table:t_im_in, index:idx_a_c(a, c) range:[1,1], keep order:true | ||
| ├─IndexRangeScan_23(Build) 4.27 cop[tikv] table:t_im_in, index:idx_b_c(b, c) range:[1,1], [2,2], keep order:true | ||
| └─TableRowIDScan_24(Probe) 5.00 cop[tikv] table:t_im_in keep order:false | ||
| select /*+ use_index_merge(t_im_in) */ * from t_im_in where a = 1 or b in (1, 2) order by c limit 5; | ||
| id a b c padding | ||
| 11 1 1 1 uncommitted | ||
| 8 1 1 5 row8 | ||
| 1 1 1 10 row1 | ||
| 2 1 2 20 row2 | ||
| 3 2 1 30 row3 | ||
| rollback; | ||
| # Case 5: PREPARE/EXECUTE with different parameter values | ||
| prepare stmt from 'select /*+ use_index_merge(t_im_in) */ * from t_im_in where a = ? or b in (1, 2) order by c limit 5'; | ||
| set @a = 1; | ||
| execute stmt using @a; | ||
| id a b c padding | ||
| 8 1 1 5 row8 | ||
| 1 1 1 10 row1 | ||
| 2 1 2 20 row2 | ||
| 3 2 1 30 row3 | ||
| 4 2 2 40 row4 | ||
| set @a = 3; | ||
| execute stmt using @a; | ||
| id a b c padding | ||
| 8 1 1 5 row8 | ||
| 1 1 1 10 row1 | ||
| 2 1 2 20 row2 | ||
| 10 3 3 25 row10 | ||
| 3 2 1 30 row3 | ||
| >>>>>>> 2310c3d99f3 (planner: support pushing Limit and TopN to individual partial paths of IndexMerge (#68772)) |
There was a problem hiding this comment.
Critical: Unresolved Git merge conflict markers present
This test result file contains unresolved merge conflict markers. The conflict shows:
- HEAD version (line 867): Empty (no conflicting content)
- Incoming version (lines 869-972): New test section for "IndexMerge with IN conditions and ORDER BY LIMIT (merge sort for partial paths)" with table creation, data insertion, and multiple test cases
The presence of these markers will cause:
- Test file parsing errors
- Integration test failures
- CI/build breakage
Action required: Resolve the conflict manually. Since HEAD has no content, this likely means accepting the incoming changes and removing the conflict markers.
🤖 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 `@tests/integrationtest/r/index_merge.result` around lines 867 - 972, The file
contains unresolved Git conflict markers (<<<<<<< HEAD, =======, >>>>>>>) around
the new test section; remove those markers and accept the incoming change block
(the entire "IndexMerge with IN conditions and ORDER BY LIMIT..." section
including table t_im_in creation, inserts, analyze, the explain outputs
referencing IndexMerge_35/IndexRangeScan_30/31/TableRowIDScan_32, UnionScan_21,
and the PREPARE/EXECUTE example) so the result file contains only the incoming
test content (no conflict lines) and retains examples like the prepare stmt and
outputs as-is.
| <<<<<<< HEAD | ||
| ======= | ||
|
|
||
| # TestIssue58361 | ||
|
|
||
| # Test non-MV index OR IndexMerge can collect usable filters (only support eq now) from the top level AND filters | ||
| drop table if exists t; | ||
| create table t (a int, b int, c int, d int, | ||
| index iab(a,b), | ||
| index iac(a,c), | ||
| index iad(a,d)); | ||
| explain format='plan_tree' select /*+ use_index_merge(t) */ * from t where a = 1 and (b = 2 or c = 3 or d = 4); | ||
|
|
||
| drop table if exists t; | ||
| create table t(a int, b int, c int, d int, | ||
| index iab(b,a), | ||
| index iac(c,a,d), | ||
| index iad(a,d)); | ||
| explain format='plan_tree' select /*+ use_index_merge(t) */ * from t where a = 1 and (b = 2 or (c = 3 and d = 4) or d = 5); | ||
|
|
||
| # Test MV index OR IndexMerge can satisfy Sort property when there are multiple indexes to choose from | ||
| drop table if exists t; | ||
| create table t (a int, b int, c int, j1 json, j2 json, | ||
| key mvi1((cast(j1 as unsigned array)), a), | ||
| key mvi2((cast(j1 as unsigned array)), b), | ||
| key mvi3((cast(j2 as unsigned array)), a), | ||
| key mvi4((cast(j2 as unsigned array)), b) ); | ||
| explain format='plan_tree' select * from t where 1 member of (j1) or 2 member of (j2) order by a; | ||
| explain format='plan_tree' select * from t where 1 member of (j1) or 2 member of (j2) order by b; | ||
| explain format='plan_tree' select * from t where json_overlaps(j1, '[1,3,5]') or 2 member of (j2) order by a desc limit 10; | ||
| explain format='plan_tree' select * from t where 1 member of (j1) or json_contains(j2, '[5]') order by b limit 10000; | ||
|
|
||
| INSERT INTO t VALUES (2, 2, 2, '[1,2,3]', '[4,5,6]'); | ||
| INSERT INTO t VALUES (5, 1, 5, '[]', '[2]'); | ||
| INSERT INTO t VALUES (10, 10, 10, '[1,2,3]', '[4,5,6]'); | ||
| INSERT INTO t VALUES (1, 2, 3, '[1,2,3]', '[4,5,6]'); | ||
| INSERT INTO t VALUES (4, 5, 6, '[7,8,5]', '[10,11,2]'); | ||
| INSERT INTO t VALUES (7, 8, 9, '[13,14,15]', '[16,17,18]'); | ||
| INSERT INTO t VALUES (10, 11, 12, '[19,20,21]', '[22,23,24]'); | ||
| INSERT INTO t VALUES (13, 14, 15, '[25,1,27]', '[2,29,30]'); | ||
| INSERT INTO t VALUES (16, 17, 18, '[31,32,33]', '[34,35,36]'); | ||
| INSERT INTO t VALUES (19, 20, 21, '[37,38,39]', '[40,2,42]'); | ||
| INSERT INTO t VALUES (22, 23, 24, '[5,44,45]', '[46,47,48]'); | ||
| INSERT INTO t VALUES (25, 26, 27, '[49,50,3]', '[52,53,54]'); | ||
| INSERT INTO t VALUES (28, 29, 30, '[5,56,57]', '[58,2,60]'); | ||
| INSERT INTO t VALUES (31, 32, 33, '[61,3,63]', '[64,65,66]'); | ||
| INSERT INTO t VALUES (34, 35, 36, '[67,68,69]', '[70,71,72]'); | ||
| INSERT INTO t VALUES (37, 38, 39, '[73,74,2]', '[76,2,2]'); | ||
|
|
||
| select /*+ use_index_merge(t) */ * from t where json_overlaps(j1, '[1,3,5]') or 2 member of (j2) order by a limit 2; | ||
| select /*+ use_index_merge(t) */ * from t where 1 member of (j1) or 2 member of (j2) order by b desc; | ||
|
|
||
| drop table if exists t; | ||
| create table t (a int, b int, c int, d int, e int, j1 json, j2 json, | ||
| key mvi1((cast(j1 as unsigned array)), a, e), | ||
| key idx1(a, b, e), | ||
| key mvi2(b, (cast(j2 as unsigned array)), e), | ||
| key idx2(d, e, c) ); | ||
|
|
||
| explain format='plan_tree' select /*+ use_index_merge(t) */ * from t where | ||
| a = 5 and | ||
| (1 member of (j1) or b = 3 or b = 4 and json_overlaps(j2, '[3,4,5]') or d = 10) | ||
| order by e; | ||
|
|
||
| explain format='plan_tree' select /*+ use_index_merge(t) */ * from t where | ||
| a = 5 and | ||
| (1 member of (j1) or b = 3 or b = 4 and json_overlaps(j2, '[3,4,5]') or d = 10) | ||
| order by e desc limit 10; | ||
|
|
||
| # TestIndexMergeSortItemsHints | ||
| # When one partial path can keep order (a=1 on idx_ac) and another cannot (b>5 on idx_bc), | ||
| # Limit should be pushed to the ordered partial path and TopN to the unordered one. | ||
| drop table if exists t; | ||
| create table t(a int, b int, c int, index idx_ac(a, c), index idx_bc(b, c)); | ||
| insert into t values(1, 1, 100), (1, 2, 50), (1, 3, 150), (6, 6, 10), (7, 7, 20), (8, 8, 30), (9, 9, 5); | ||
| explain format='plan_tree' select /*+ use_index_merge(t, idx_ac, idx_bc) */ * from t where a = 1 or b > 5 order by c limit 3; | ||
| select /*+ use_index_merge(t, idx_ac, idx_bc) */ * from t where a = 1 or b > 5 order by c limit 3; | ||
| explain format='plan_tree' select /*+ use_index_merge(t, idx_ac, idx_bc) */ * from t where a = 1 or b > 5 order by c limit 3 offset 1; | ||
| select /*+ use_index_merge(t, idx_ac, idx_bc) */ * from t where a = 1 or b > 5 order by c limit 3 offset 1; | ||
| >>>>>>> 2310c3d99f3 (planner: support pushing Limit and TopN to individual partial paths of IndexMerge (#68772)) |
There was a problem hiding this comment.
Critical: Unresolved Git merge conflict markers present
This test input file contains unresolved merge conflict markers. The conflict shows:
- HEAD version (line 520): Empty (no conflicting content)
- Incoming version (lines 522-599): New test sections:
TestIssue58361: Tests for non-MV index OR IndexMerge filter collection and MV index OR IndexMerge sort satisfactionTestIndexMergeSortItemsHints: Tests for Limit/TopN pushdown to ordered/unordered partial paths
The presence of these markers will cause:
- Test file parsing errors
- Integration test execution failures
- CI pipeline breakage
Action required: Resolve the conflict manually. Since HEAD is empty, this likely means accepting the incoming test cases and removing the conflict markers. Ensure consistency with the corresponding .result file resolution.
🤖 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 `@tests/integrationtest/t/planner/core/indexmerge_path.test` around lines 520 -
599, Resolve the unresolved git conflict markers by removing the conflict
delimiters (<<<<<<< HEAD, =======, >>>>>>> 2310c3d99f3) and keeping the incoming
test blocks under TestIssue58361 and TestIndexMergeSortItemsHints (i.e., accept
the incoming content for those tests), then update the corresponding .result
expectations to match the added tests and run the test suite to verify
parsing/execution succeed; locate the test sections by the unique test headings
"TestIssue58361" and "TestIndexMergeSortItemsHints" to apply the fix.
2 participants
This is an automated cherry-pick of #68772
What problem does this PR solve?
Issue Number: ref #65712 close #68773
Problem Summary:
For queries like
SELECT * FROM t WHERE (a = 1 OR b > 5) ORDER BY c LIMIT 5, where indexesidx_ac(a, c)andidx_bc(b, c)are available:a = 1partial path onidx_accan satisfyORDER BY c(single value on the leading columna).b > 5partial path onidx_bccannot satisfyORDER BY c(range scan on the leading columnb).Previously, if not all partial paths of an IndexMerge could satisfy the
ORDER BY, the plan would fall back to a globalTopNwithout any pushdown, scanning all matching rows before sorting.This is Solution 2 described in the issue.
What changed and how does it work?
SortItemsHintsis added toPhysicalProperty. WhenTopNsits directly above aDataSource, theORDER BYcolumns are passed as advisory sort items through the property.DataSourceuses these advisory sort items when selecting alternatives for OR-type IndexMerge, preferring partial paths that can satisfy the sort order.Limitis pushed to the matching ones whileTopNis pushed to the rest.Key changes:
PhysicalProperty: newSortItemsHintsfield for advisory sort preferences (pkg/planner/property/physical_property.go)getPhysTopN: setsSortItemsHintswhen the child is aDataSource(pkg/planner/core/operator/physicalop/physical_topn.go)candidatePath: newmatchWithAdvisorySortItemsflag (true when matching used SortItemsHints) andpartialPathMatchResultsfield storing each partial path'smatchPropertyresult, replacing the simpler boolean satisfaction slice (pkg/planner/core/find_best_task.go)CopTask: newIdxMergeMatchWithAdvisorySortItemsflag andIdxMergePartPlansMatchResultsfield storing each partial path'smatchPropertyresult (pkg/planner/core/operator/physicalop/task_base.go)matchPropForIndexMergeAlternatives/isMatchPropForIndexMerge: collect per-pathmatchPropertyresults inline during matching rather than recomputing afterwards (pkg/planner/core/find_best_task.go)convertToIndexMergeScan: passes each partial path's own match result (and the effective prop — hints-based or original) when building partial scans (pkg/planner/core/find_best_task.go)attach2Task4PhysicalTopN: newhandleAdvisorySortItemsForIndexMergethat pushesLimitto satisfying partial paths andTopNto others. Only applies when some (but not all) paths satisfy, since the existingLimitpushdown handles the all-satisfying case. (pkg/planner/core/task.go)Check List
Tests
Side effects
Documentation
Release note
Summary by CodeRabbit
Release Notes
New Features
ORDER BY LIMIT, enabling the optimizer to push limit operations into individual merge paths for improved performance.Tests