From 48ec282557c86dd88c0c543ab0b16ae3d696b3cc Mon Sep 17 00:00:00 2001 From: time-and-fate <25057648+time-and-fate@users.noreply.github.com> Date: Mon, 15 Nov 2021 15:26:57 +0800 Subject: [PATCH 1/3] fix --- planner/core/task.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/planner/core/task.go b/planner/core/task.go index f3d0d509288b9..b54adf20601ed 100644 --- a/planner/core/task.go +++ b/planner/core/task.go @@ -1295,6 +1295,20 @@ func (p *PhysicalTopN) getPushedDownTopN(childPlan PhysicalPlan) *PhysicalTopN { return topN } +func (p *PhysicalTopN) canPushToIndexPlan(indexPlan PhysicalPlan, byItemCols []*expression.Column) bool { + schema := indexPlan.Schema() + for _, col := range byItemCols { + pos := schema.ColumnIndex(col) + if pos == -1 { + return false + } + if schema.Columns[pos].IsPrefix == true { + return false + } + } + return true +} + func (p *PhysicalTopN) attach2Task(tasks ...task) task { t := tasks[0].copy() inputCount := t.count() @@ -1307,7 +1321,7 @@ func (p *PhysicalTopN) attach2Task(tasks ...task) task { // If all columns in topN are from index plan, we push it to index plan, otherwise we finish the index plan and // push it to table plan. var pushedDownTopN *PhysicalTopN - if !copTask.indexPlanFinished && len(copTask.indexPlan.Schema().ColumnsIndices(cols)) > 0 { + if !copTask.indexPlanFinished && p.canPushToIndexPlan(copTask.indexPlan, cols) { pushedDownTopN = p.getPushedDownTopN(copTask.indexPlan) copTask.indexPlan = pushedDownTopN } else { From c60bf4687d5fd9f96e1ce8c86f12934d4c87f420 Mon Sep 17 00:00:00 2001 From: time-and-fate <25057648+time-and-fate@users.noreply.github.com> Date: Mon, 15 Nov 2021 15:50:16 +0800 Subject: [PATCH 2/3] add test and fix unnecessary code --- planner/core/integration_test.go | 36 ++++++++++++++++++++++++++++++++ planner/core/task.go | 2 +- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/planner/core/integration_test.go b/planner/core/integration_test.go index 63b3a44f2e3dc..6395df84403a8 100644 --- a/planner/core/integration_test.go +++ b/planner/core/integration_test.go @@ -4750,3 +4750,39 @@ func (s *testIntegrationSerialSuite) TestRejectSortForMPP(c *C) { res.Check(testkit.Rows(output[i].Plan...)) } } + +func (s *testIntegrationSuite) TestIssues29711(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + + tk.MustExec("drop table if exists tbl_29711") + tk.MustExec("CREATE TABLE `tbl_29711` (" + + "`col_250` text COLLATE utf8_unicode_ci NOT NULL," + + "`col_251` enum('Alice','Bob','Charlie','David') COLLATE utf8_unicode_ci NOT NULL DEFAULT 'Charlie'," + + "PRIMARY KEY (`col_251`,`col_250`(1)) NONCLUSTERED);") + tk.MustQuery("explain format=brief " + + "select col_250,col_251 from tbl_29711 where col_251 between 'Bob' and 'David' order by col_250,col_251 limit 6;"). + Check(testkit.Rows( + "TopN 6.00 root test.tbl_29711.col_250, test.tbl_29711.col_251, offset:0, count:6", + "└─IndexLookUp 6.00 root ", + " ├─IndexRangeScan(Build) 30.00 cop[tikv] table:tbl_29711, index:PRIMARY(col_251, col_250) range:[\"Bob\",\"Bob\"], [\"Charlie\",\"Charlie\"], [\"David\",\"David\"], keep order:false, stats:pseudo", + " └─TopN(Probe) 6.00 cop[tikv] test.tbl_29711.col_250, test.tbl_29711.col_251, offset:0, count:6", + " └─TableRowIDScan 30.00 cop[tikv] table:tbl_29711 keep order:false, stats:pseudo", + )) + + tk.MustExec("drop table if exists t29711") + tk.MustExec("CREATE TABLE `t29711` (" + + "`a` varchar(10) DEFAULT NULL," + + "`b` int(11) DEFAULT NULL," + + "`c` int(11) DEFAULT NULL," + + "KEY `ia` (`a`(2)))") + tk.MustQuery("explain format=brief select * from t29711 use index (ia) order by a limit 10;"). + Check(testkit.Rows( + "TopN 10.00 root test.t29711.a, offset:0, count:10", + "└─IndexLookUp 10.00 root ", + " ├─IndexFullScan(Build) 10000.00 cop[tikv] table:t29711, index:ia(a) keep order:false, stats:pseudo", + " └─TopN(Probe) 10.00 cop[tikv] test.t29711.a, offset:0, count:10", + " └─TableRowIDScan 10000.00 cop[tikv] table:t29711 keep order:false, stats:pseudo", + )) + +} diff --git a/planner/core/task.go b/planner/core/task.go index b54adf20601ed..d4b68a9a04610 100644 --- a/planner/core/task.go +++ b/planner/core/task.go @@ -1302,7 +1302,7 @@ func (p *PhysicalTopN) canPushToIndexPlan(indexPlan PhysicalPlan, byItemCols []* if pos == -1 { return false } - if schema.Columns[pos].IsPrefix == true { + if schema.Columns[pos].IsPrefix { return false } } From a443259a48084db48ac4ef58c905687b532e71ae Mon Sep 17 00:00:00 2001 From: time-and-fate <25057648+time-and-fate@users.noreply.github.com> Date: Mon, 15 Nov 2021 16:43:03 +0800 Subject: [PATCH 3/3] add comments --- planner/core/task.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/planner/core/task.go b/planner/core/task.go index d4b68a9a04610..d0ed6bf0b35ec 100644 --- a/planner/core/task.go +++ b/planner/core/task.go @@ -1295,6 +1295,9 @@ func (p *PhysicalTopN) getPushedDownTopN(childPlan PhysicalPlan) *PhysicalTopN { return topN } +// canPushToIndexPlan checks if this TopN can be pushed to the index side of copTask. +// It can be pushed to the index side when all columns used by ByItems are available from the index side and +// there's no prefix index column. func (p *PhysicalTopN) canPushToIndexPlan(indexPlan PhysicalPlan, byItemCols []*expression.Column) bool { schema := indexPlan.Schema() for _, col := range byItemCols {