From c400c508f781507e5957d5a1695d7a6b74556ab5 Mon Sep 17 00:00:00 2001 From: AilinKid <314806019@qq.com> Date: Mon, 29 Apr 2024 16:56:48 +0800 Subject: [PATCH 1/4] fix index merge should't push partial limit down when index plans are keep ordered Signed-off-by: AilinKid <314806019@qq.com> --- pkg/planner/core/find_best_task.go | 3 +++ pkg/planner/core/task.go | 12 +++++++++--- .../physicalplantest/physical_plan.result | 15 +++++++++++++++ .../casetest/physicalplantest/physical_plan.test | 12 ++++++++++++ 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/pkg/planner/core/find_best_task.go b/pkg/planner/core/find_best_task.go index b3fc8a283d845..892af6cb52df4 100644 --- a/pkg/planner/core/find_best_task.go +++ b/pkg/planner/core/find_best_task.go @@ -1374,6 +1374,9 @@ func (ds *DataSource) findBestTask(prop *property.PhysicalProperty, planCounter } t = invalidTask + if strings.Contains(ds.SCtx().GetSessionVars().StmtCtx.OriginalSQL, "explain select max(col_304) from (select /*+ use_index_merge( tbl_43 ) */") { + fmt.Println(1) + } candidates := ds.skylinePruning(prop) pruningInfo := ds.getPruningInfo(candidates, prop) defer func() { diff --git a/pkg/planner/core/task.go b/pkg/planner/core/task.go index 966d4ec4172c0..a140ccb233d26 100644 --- a/pkg/planner/core/task.go +++ b/pkg/planner/core/task.go @@ -875,10 +875,12 @@ func (p *PhysicalLimit) attach2Task(tasks ...task) task { } else if !cop.idxMergeIsIntersection { // We only support push part of the order prop down to index merge build case. if len(cop.rootTaskConds) == 0 { - if cop.indexPlanFinished { - // when the index plan is finished, sink the limit to the index merge table side. + // since issues/52947, we shouldn't push down part limit down to table plans, because + // the handles we pruned/limited is not according to index-order (internal handles reorder should be considered) + if cop.indexPlanFinished && !cop.keepOrder { + // when the index plan is finished and index plan is not ordered, sink the limit to the index merge table side. suspendLimitAboveTablePlan() - } else { + } else if !cop.indexPlanFinished { // cop.indexPlanFinished = false indicates the table side is a pure table-scan, sink the limit to the index merge index side. newCount := p.Offset + p.Count limitChildren := make([]PhysicalPlan, 0, len(cop.idxMergePartPlans)) @@ -893,6 +895,10 @@ func (p *PhysicalLimit) attach2Task(tasks ...task) task { cop.idxMergePartPlans = limitChildren t = cop.convertToRootTask(p.SCtx()) sunk = p.sinkIntoIndexMerge(t) + } else { + // when there are some limitations, just sink the limit upon the index merge reader. + t = cop.ConvertToRootTask(p.SCtx()) + sunk = p.sinkIntoIndexMerge(t) } } else { // when there are some root conditions, just sink the limit upon the index merge reader. diff --git a/tests/integrationtest/r/planner/core/casetest/physicalplantest/physical_plan.result b/tests/integrationtest/r/planner/core/casetest/physicalplantest/physical_plan.result index e50d6a0705f3a..f6977b8e7677f 100644 --- a/tests/integrationtest/r/planner/core/casetest/physicalplantest/physical_plan.result +++ b/tests/integrationtest/r/planner/core/casetest/physicalplantest/physical_plan.result @@ -3728,3 +3728,18 @@ IndexMerge_20 1.00 root type: union, limit embedded(offset:0, count:1) └─TableRowIDScan_13(Probe) 1.00 cop[tikv] table:t3 keep order:false, stats:pseudo show warnings; Level Code Message +CREATE TABLE `tbl_43` ( +`col_304` binary(207) NOT NULL DEFAULT 'eIenHx\0\0\0\0\0\0\0\0\0\0\0\0', +PRIMARY KEY (`col_304`) /*T![clustered_index] CLUSTERED */, +UNIQUE KEY `idx_259` (`col_304`(5)), +UNIQUE KEY `idx_260` (`col_304`(2)), +KEY `idx_261` (`col_304`), +UNIQUE KEY `idx_262` (`col_304`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin; +insert into tbl_43 values("BCmuENPHzSOIMJLPB"),("LDOdXZYpOR"),("R"),("TloTqcHhdgpwvMsSoJ"),("UajN"),("mAwLZbiyq"),("swLIoWa"); +select min(col_304) from (select /*+ use_index_merge( tbl_43 ) */ * from tbl_43 where not( tbl_43.col_304 between 'YEpfYfPVvhMlHGHSMKm' and 'PE' ) or tbl_43.col_304 in ( 'LUBGzGMA' ) and tbl_43.col_304 between 'HpsjfuSReCwBoh' and 'fta' or not( tbl_43.col_304 between 'MFWmuOsoyDv' and 'TSeMYpDXnFIyp' ) order by col_304) x; +min(col_304) +BCmuENPHzSOIMJLPB +select max(col_304) from (select /*+ use_index_merge( tbl_43 ) */ * from tbl_43 where not( tbl_43.col_304 between 'YEpfYfPVvhMlHGHSMKm' and 'PE' ) or tbl_43.col_304 in ( 'LUBGzGMA' ) and tbl_43.col_304 between 'HpsjfuSReCwBoh' and 'fta' or not( tbl_43.col_304 between 'MFWmuOsoyDv' and 'TSeMYpDXnFIyp' ) order by col_304) x; +max(col_304) +swLIoWa diff --git a/tests/integrationtest/t/planner/core/casetest/physicalplantest/physical_plan.test b/tests/integrationtest/t/planner/core/casetest/physicalplantest/physical_plan.test index 3e28f72db0e17..e1ae971e63fcb 100644 --- a/tests/integrationtest/t/planner/core/casetest/physicalplantest/physical_plan.test +++ b/tests/integrationtest/t/planner/core/casetest/physicalplantest/physical_plan.test @@ -992,3 +992,15 @@ set tidb_cost_model_version=DEFAULT; explain select /*+ USE_INDEX_MERGE(t3, aid_c1, aid_c2) */ * from t3 where (aid = 1 and c1='aaa') or (aid = 1 and c2='bbb') limit 1; show warnings; +# TestIndexMergeIssue52947 +CREATE TABLE `tbl_43` ( + `col_304` binary(207) NOT NULL DEFAULT 'eIenHx\0\0\0\0\0\0\0\0\0\0\0\0', + PRIMARY KEY (`col_304`) /*T![clustered_index] CLUSTERED */, + UNIQUE KEY `idx_259` (`col_304`(5)), + UNIQUE KEY `idx_260` (`col_304`(2)), + KEY `idx_261` (`col_304`), + UNIQUE KEY `idx_262` (`col_304`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin; +insert into tbl_43 values("BCmuENPHzSOIMJLPB"),("LDOdXZYpOR"),("R"),("TloTqcHhdgpwvMsSoJ"),("UajN"),("mAwLZbiyq"),("swLIoWa"); +select min(col_304) from (select /*+ use_index_merge( tbl_43 ) */ * from tbl_43 where not( tbl_43.col_304 between 'YEpfYfPVvhMlHGHSMKm' and 'PE' ) or tbl_43.col_304 in ( 'LUBGzGMA' ) and tbl_43.col_304 between 'HpsjfuSReCwBoh' and 'fta' or not( tbl_43.col_304 between 'MFWmuOsoyDv' and 'TSeMYpDXnFIyp' ) order by col_304) x; +select max(col_304) from (select /*+ use_index_merge( tbl_43 ) */ * from tbl_43 where not( tbl_43.col_304 between 'YEpfYfPVvhMlHGHSMKm' and 'PE' ) or tbl_43.col_304 in ( 'LUBGzGMA' ) and tbl_43.col_304 between 'HpsjfuSReCwBoh' and 'fta' or not( tbl_43.col_304 between 'MFWmuOsoyDv' and 'TSeMYpDXnFIyp' ) order by col_304) x; \ No newline at end of file From 0e941af33a45c754961dfd6d8893d8c247997aaa Mon Sep 17 00:00:00 2001 From: AilinKid <314806019@qq.com> Date: Mon, 29 Apr 2024 17:00:12 +0800 Subject: [PATCH 2/4] . Signed-off-by: AilinKid <314806019@qq.com> --- pkg/planner/core/find_best_task.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/planner/core/find_best_task.go b/pkg/planner/core/find_best_task.go index 892af6cb52df4..b3fc8a283d845 100644 --- a/pkg/planner/core/find_best_task.go +++ b/pkg/planner/core/find_best_task.go @@ -1374,9 +1374,6 @@ func (ds *DataSource) findBestTask(prop *property.PhysicalProperty, planCounter } t = invalidTask - if strings.Contains(ds.SCtx().GetSessionVars().StmtCtx.OriginalSQL, "explain select max(col_304) from (select /*+ use_index_merge( tbl_43 ) */") { - fmt.Println(1) - } candidates := ds.skylinePruning(prop) pruningInfo := ds.getPruningInfo(candidates, prop) defer func() { From 9b9769999508eeeec2f5cbf7ab470a2b1680e71c Mon Sep 17 00:00:00 2001 From: AilinKid <314806019@qq.com> Date: Mon, 6 May 2024 16:33:22 +0800 Subject: [PATCH 3/4] . Signed-off-by: AilinKid <314806019@qq.com> --- pkg/planner/core/task.go | 4 +-- .../physicalplantest/physical_plan.result | 26 +++++++++++++++++++ .../physicalplantest/physical_plan.test | 2 ++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/pkg/planner/core/task.go b/pkg/planner/core/task.go index a140ccb233d26..48137adec60f9 100644 --- a/pkg/planner/core/task.go +++ b/pkg/planner/core/task.go @@ -875,8 +875,8 @@ func (p *PhysicalLimit) attach2Task(tasks ...task) task { } else if !cop.idxMergeIsIntersection { // We only support push part of the order prop down to index merge build case. if len(cop.rootTaskConds) == 0 { - // since issues/52947, we shouldn't push down part limit down to table plans, because - // the handles we pruned/limited is not according to index-order (internal handles reorder should be considered) + // For double read which requires order being kept, the limit cannot be pushed down to the table side, + // because handles would be reordered before being sent to table scan. if cop.indexPlanFinished && !cop.keepOrder { // when the index plan is finished and index plan is not ordered, sink the limit to the index merge table side. suspendLimitAboveTablePlan() diff --git a/tests/integrationtest/r/planner/core/casetest/physicalplantest/physical_plan.result b/tests/integrationtest/r/planner/core/casetest/physicalplantest/physical_plan.result index f6977b8e7677f..52d57aab7c90e 100644 --- a/tests/integrationtest/r/planner/core/casetest/physicalplantest/physical_plan.result +++ b/tests/integrationtest/r/planner/core/casetest/physicalplantest/physical_plan.result @@ -3737,9 +3737,35 @@ KEY `idx_261` (`col_304`), UNIQUE KEY `idx_262` (`col_304`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin; insert into tbl_43 values("BCmuENPHzSOIMJLPB"),("LDOdXZYpOR"),("R"),("TloTqcHhdgpwvMsSoJ"),("UajN"),("mAwLZbiyq"),("swLIoWa"); +explain format = 'brief' select min(col_304) from (select /*+ use_index_merge( tbl_43 ) */ * from tbl_43 where not( tbl_43.col_304 between 'YEpfYfPVvhMlHGHSMKm' and 'PE' ) or tbl_43.col_304 in ( 'LUBGzGMA' ) and tbl_43.col_304 between 'HpsjfuSReCwBoh' and 'fta' or not( tbl_43.col_304 between 'MFWmuOsoyDv' and 'TSeMYpDXnFIyp' ) order by col_304) x; +id estRows task access object operator info +StreamAgg 1.00 root funcs:min(planner__core__casetest__physicalplantest__physical_plan.tbl_43.col_304)->Column#2 +└─Limit 1.00 root offset:0, count:1 + └─IndexMerge 1.00 root type: union + ├─Selection(Build) 0.00 cop[tikv] 1 + │ └─TableRangeScan 0.00 cop[tikv] table:tbl_43 range:["LUBGzGMA","LUBGzGMA"], keep order:true, stats:pseudo + ├─IndexRangeScan(Build) 0.42 cop[tikv] table:tbl_43, index:idx_261(col_304) range:[-inf,"YEpfYfPVvhMlHGHSMKm"), keep order:true, stats:pseudo + ├─IndexRangeScan(Build) 0.42 cop[tikv] table:tbl_43, index:idx_262(col_304) range:("PE",+inf], keep order:true, stats:pseudo + ├─TableRangeScan(Build) 0.42 cop[tikv] table:tbl_43 range:[-inf,"MFWmuOsoyDv"), keep order:true, stats:pseudo + ├─TableRangeScan(Build) 0.42 cop[tikv] table:tbl_43 range:("TSeMYpDXnFIyp",+inf], keep order:true, stats:pseudo + └─Selection(Probe) 1.00 cop[tikv] or(or(lt(planner__core__casetest__physicalplantest__physical_plan.tbl_43.col_304, "YEpfYfPVvhMlHGHSMKm"), gt(planner__core__casetest__physicalplantest__physical_plan.tbl_43.col_304, "PE")), or(and(eq(planner__core__casetest__physicalplantest__physical_plan.tbl_43.col_304, "LUBGzGMA"), 1), or(lt(planner__core__casetest__physicalplantest__physical_plan.tbl_43.col_304, "MFWmuOsoyDv"), gt(planner__core__casetest__physicalplantest__physical_plan.tbl_43.col_304, "TSeMYpDXnFIyp")))) + └─TableRowIDScan 1.25 cop[tikv] table:tbl_43 keep order:false, stats:pseudo select min(col_304) from (select /*+ use_index_merge( tbl_43 ) */ * from tbl_43 where not( tbl_43.col_304 between 'YEpfYfPVvhMlHGHSMKm' and 'PE' ) or tbl_43.col_304 in ( 'LUBGzGMA' ) and tbl_43.col_304 between 'HpsjfuSReCwBoh' and 'fta' or not( tbl_43.col_304 between 'MFWmuOsoyDv' and 'TSeMYpDXnFIyp' ) order by col_304) x; min(col_304) BCmuENPHzSOIMJLPB +explain format = 'brief' select max(col_304) from (select /*+ use_index_merge( tbl_43 ) */ * from tbl_43 where not( tbl_43.col_304 between 'YEpfYfPVvhMlHGHSMKm' and 'PE' ) or tbl_43.col_304 in ( 'LUBGzGMA' ) and tbl_43.col_304 between 'HpsjfuSReCwBoh' and 'fta' or not( tbl_43.col_304 between 'MFWmuOsoyDv' and 'TSeMYpDXnFIyp' ) order by col_304) x; +id estRows task access object operator info +StreamAgg 1.00 root funcs:max(planner__core__casetest__physicalplantest__physical_plan.tbl_43.col_304)->Column#2 +└─Limit 1.00 root offset:0, count:1 + └─IndexMerge 1.00 root type: union + ├─Selection(Build) 0.00 cop[tikv] 1 + │ └─TableRangeScan 0.00 cop[tikv] table:tbl_43 range:["LUBGzGMA","LUBGzGMA"], keep order:true, desc, stats:pseudo + ├─IndexRangeScan(Build) 0.42 cop[tikv] table:tbl_43, index:idx_261(col_304) range:[-inf,"YEpfYfPVvhMlHGHSMKm"), keep order:true, desc, stats:pseudo + ├─IndexRangeScan(Build) 0.42 cop[tikv] table:tbl_43, index:idx_262(col_304) range:("PE",+inf], keep order:true, desc, stats:pseudo + ├─TableRangeScan(Build) 0.42 cop[tikv] table:tbl_43 range:[-inf,"MFWmuOsoyDv"), keep order:true, desc, stats:pseudo + ├─TableRangeScan(Build) 0.42 cop[tikv] table:tbl_43 range:("TSeMYpDXnFIyp",+inf], keep order:true, desc, stats:pseudo + └─Selection(Probe) 1.00 cop[tikv] or(or(lt(planner__core__casetest__physicalplantest__physical_plan.tbl_43.col_304, "YEpfYfPVvhMlHGHSMKm"), gt(planner__core__casetest__physicalplantest__physical_plan.tbl_43.col_304, "PE")), or(and(eq(planner__core__casetest__physicalplantest__physical_plan.tbl_43.col_304, "LUBGzGMA"), 1), or(lt(planner__core__casetest__physicalplantest__physical_plan.tbl_43.col_304, "MFWmuOsoyDv"), gt(planner__core__casetest__physicalplantest__physical_plan.tbl_43.col_304, "TSeMYpDXnFIyp")))) + └─TableRowIDScan 1.25 cop[tikv] table:tbl_43 keep order:false, stats:pseudo select max(col_304) from (select /*+ use_index_merge( tbl_43 ) */ * from tbl_43 where not( tbl_43.col_304 between 'YEpfYfPVvhMlHGHSMKm' and 'PE' ) or tbl_43.col_304 in ( 'LUBGzGMA' ) and tbl_43.col_304 between 'HpsjfuSReCwBoh' and 'fta' or not( tbl_43.col_304 between 'MFWmuOsoyDv' and 'TSeMYpDXnFIyp' ) order by col_304) x; max(col_304) swLIoWa diff --git a/tests/integrationtest/t/planner/core/casetest/physicalplantest/physical_plan.test b/tests/integrationtest/t/planner/core/casetest/physicalplantest/physical_plan.test index e1ae971e63fcb..ae30fe07ab250 100644 --- a/tests/integrationtest/t/planner/core/casetest/physicalplantest/physical_plan.test +++ b/tests/integrationtest/t/planner/core/casetest/physicalplantest/physical_plan.test @@ -1002,5 +1002,7 @@ CREATE TABLE `tbl_43` ( UNIQUE KEY `idx_262` (`col_304`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin; insert into tbl_43 values("BCmuENPHzSOIMJLPB"),("LDOdXZYpOR"),("R"),("TloTqcHhdgpwvMsSoJ"),("UajN"),("mAwLZbiyq"),("swLIoWa"); +explain format = 'brief' select min(col_304) from (select /*+ use_index_merge( tbl_43 ) */ * from tbl_43 where not( tbl_43.col_304 between 'YEpfYfPVvhMlHGHSMKm' and 'PE' ) or tbl_43.col_304 in ( 'LUBGzGMA' ) and tbl_43.col_304 between 'HpsjfuSReCwBoh' and 'fta' or not( tbl_43.col_304 between 'MFWmuOsoyDv' and 'TSeMYpDXnFIyp' ) order by col_304) x; select min(col_304) from (select /*+ use_index_merge( tbl_43 ) */ * from tbl_43 where not( tbl_43.col_304 between 'YEpfYfPVvhMlHGHSMKm' and 'PE' ) or tbl_43.col_304 in ( 'LUBGzGMA' ) and tbl_43.col_304 between 'HpsjfuSReCwBoh' and 'fta' or not( tbl_43.col_304 between 'MFWmuOsoyDv' and 'TSeMYpDXnFIyp' ) order by col_304) x; +explain format = 'brief' select max(col_304) from (select /*+ use_index_merge( tbl_43 ) */ * from tbl_43 where not( tbl_43.col_304 between 'YEpfYfPVvhMlHGHSMKm' and 'PE' ) or tbl_43.col_304 in ( 'LUBGzGMA' ) and tbl_43.col_304 between 'HpsjfuSReCwBoh' and 'fta' or not( tbl_43.col_304 between 'MFWmuOsoyDv' and 'TSeMYpDXnFIyp' ) order by col_304) x; select max(col_304) from (select /*+ use_index_merge( tbl_43 ) */ * from tbl_43 where not( tbl_43.col_304 between 'YEpfYfPVvhMlHGHSMKm' and 'PE' ) or tbl_43.col_304 in ( 'LUBGzGMA' ) and tbl_43.col_304 between 'HpsjfuSReCwBoh' and 'fta' or not( tbl_43.col_304 between 'MFWmuOsoyDv' and 'TSeMYpDXnFIyp' ) order by col_304) x; \ No newline at end of file From c32f4c7a7377ebf234539b36d17dd6c3e29ba11a Mon Sep 17 00:00:00 2001 From: AilinKid <314806019@qq.com> Date: Fri, 10 May 2024 11:53:40 +0800 Subject: [PATCH 4/4] resolve conflicts Signed-off-by: AilinKid <314806019@qq.com> --- pkg/planner/core/task.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/planner/core/task.go b/pkg/planner/core/task.go index 48137adec60f9..62e0d53c172ee 100644 --- a/pkg/planner/core/task.go +++ b/pkg/planner/core/task.go @@ -897,7 +897,7 @@ func (p *PhysicalLimit) attach2Task(tasks ...task) task { sunk = p.sinkIntoIndexMerge(t) } else { // when there are some limitations, just sink the limit upon the index merge reader. - t = cop.ConvertToRootTask(p.SCtx()) + t = cop.convertToRootTask(p.SCtx()) sunk = p.sinkIntoIndexMerge(t) } } else {