From de862fc6206072eced41bb6d006e8c06ff9048ee Mon Sep 17 00:00:00 2001 From: "Zhuomin(Charming) Liu" Date: Mon, 2 Dec 2019 13:44:28 +0800 Subject: [PATCH] planner: do not push keep descending order to tiflash (#13572) --- cmd/explaintest/r/access_tiflash.result | 15 +++++++++++++++ cmd/explaintest/t/access_tiflash.test | 9 ++++++++- planner/core/find_best_task.go | 19 ++++++++++++------- planner/core/logical_plan_builder.go | 21 +++++++++++++++++---- 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/cmd/explaintest/r/access_tiflash.result b/cmd/explaintest/r/access_tiflash.result index 3bf05e9077..d44565ca9e 100644 --- a/cmd/explaintest/r/access_tiflash.result +++ b/cmd/explaintest/r/access_tiflash.result @@ -40,3 +40,18 @@ id count task operator info TableReader_7 44.00 root data:Selection_6 └─Selection_6 44.00 cop[tiflash] or(and(gt(test.tt.a, 1), lt(test.tt.a, 20)), and(ge(test.tt.a, 30), lt(test.tt.a, 55))) └─TableScan_5 44.00 cop[tiflash] table:tt, range:[-inf,+inf], keep order:false, stats:pseudo +drop table if exists ttt; +create table ttt (a int, primary key (a desc)); +desc select * from ttt order by ttt.a desc; +id count task operator info +TableReader_11 10000.00 root data:TableScan_10 +└─TableScan_10 10000.00 cop[tikv] table:ttt, range:[-inf,+inf], keep order:true, desc, stats:pseudo +desc select /*+ read_from_storage(tiflash[ttt]) */ * from ttt order by ttt.a desc; +id count task operator info +Sort_4 10000.00 root test.ttt.a:desc +└─TableReader_8 10000.00 root data:TableScan_7 + └─TableScan_7 10000.00 cop[tiflash] table:ttt, range:[-inf,+inf], keep order:false, stats:pseudo +desc select /*+ read_from_storage(tiflash[ttt]) */ * from ttt order by ttt.a; +id count task operator info +TableReader_11 10000.00 root data:TableScan_10 +└─TableScan_10 10000.00 cop[tiflash] table:ttt, range:[-inf,+inf], keep order:true, stats:pseudo diff --git a/cmd/explaintest/t/access_tiflash.test b/cmd/explaintest/t/access_tiflash.test index 101892d7f4..76604cdf0b 100644 --- a/cmd/explaintest/t/access_tiflash.test +++ b/cmd/explaintest/t/access_tiflash.test @@ -15,4 +15,11 @@ desc select /*+ read_from_storage(tiflash[t]) */ sum(isnull(a)) from t; create table tt(a int, b int, primary key(a)); desc select * from tt where (tt.a > 1 and tt.a < 20) or (tt.a >= 30 and tt.a < 55); -desc select /*+ read_from_storage(tiflash[tt]) */ * from tt where (tt.a > 1 and tt.a < 20) or (tt.a >= 30 and tt.a < 55); \ No newline at end of file +desc select /*+ read_from_storage(tiflash[tt]) */ * from tt where (tt.a > 1 and tt.a < 20) or (tt.a >= 30 and tt.a < 55); + +drop table if exists ttt; +create table ttt (a int, primary key (a desc)); + +desc select * from ttt order by ttt.a desc; +desc select /*+ read_from_storage(tiflash[ttt]) */ * from ttt order by ttt.a desc; +desc select /*+ read_from_storage(tiflash[ttt]) */ * from ttt order by ttt.a; diff --git a/planner/core/find_best_task.go b/planner/core/find_best_task.go index 886064fea4..7851582a80 100644 --- a/planner/core/find_best_task.go +++ b/planner/core/find_best_task.go @@ -278,7 +278,12 @@ func compareCandidates(lhs, rhs *candidatePath) int { func (ds *DataSource) getTableCandidate(path *accessPath, prop *property.PhysicalProperty) *candidatePath { candidate := &candidatePath{path: path} pkCol := ds.getPKIsHandleCol() - candidate.isMatchProp = len(prop.Items) == 1 && pkCol != nil && prop.Items[0].Col.Equal(nil, pkCol) + if len(prop.Items) == 1 && pkCol != nil { + candidate.isMatchProp = prop.Items[0].Col.Equal(nil, pkCol) + if path.storeType == kv.TiFlash { + candidate.isMatchProp = candidate.isMatchProp && !prop.Items[0].Desc + } + } candidate.columnSet = expression.ExtractColumnSet(path.accessConds) candidate.isSingleScan = true return candidate @@ -430,6 +435,12 @@ func (ds *DataSource) findBestTask(prop *property.PhysicalProperty) (t task, err }, nil } if path.isTablePath { + if ds.preferStoreType&preferTiFlash != 0 && path.storeType == kv.TiKV { + continue + } + if ds.preferStoreType&preferTiKV != 0 && path.storeType == kv.TiFlash { + continue + } tblTask, err := ds.convertToTableScan(prop, candidate) if err != nil { return nil, err @@ -1029,12 +1040,6 @@ func (ds *DataSource) getOriginalPhysicalTableScan(prop *property.PhysicalProper filterCondition: path.tableFilters, StoreType: path.storeType, }.Init(ds.ctx, ds.blockOffset) - if ds.preferStoreType&preferTiFlash != 0 { - ts.StoreType = kv.TiFlash - } - if ds.preferStoreType&preferTiKV != 0 { - ts.StoreType = kv.TiKV - } if ts.StoreType == kv.TiFlash { // Append the AccessCondition to filterCondition because TiFlash only support full range scan for each // region, do not reset ts.Ranges as it will help prune regions during `buildCopTasks` diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index 25477a1998..78bf6e6968 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -35,6 +35,7 @@ import ( "github.com/pingcap/tidb/expression" "github.com/pingcap/tidb/expression/aggregation" "github.com/pingcap/tidb/infoschema" + "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/metrics" "github.com/pingcap/tidb/planner/property" "github.com/pingcap/tidb/privilege" @@ -436,10 +437,10 @@ func (ds *DataSource) setPreferredStoreType(hintInfo *tableHintInfo) { } else { alias = &hintTableInfo{dbName: ds.DBName, tblName: ds.tableInfo.Name, selectOffset: ds.SelectBlockOffset()} } - if hintInfo.ifPreferTiFlash(alias) { - ds.preferStoreType |= preferTiFlash - } if hintInfo.ifPreferTiKV(alias) { + ds.preferStoreType |= preferTiKV + } + if hintInfo.ifPreferTiFlash(alias) { if ds.preferStoreType != 0 { errMsg := fmt.Sprintf("Storage hints are conflict, you can only specify one storage type of table %s.%s", alias.dbName.L, alias.tblName.L) @@ -448,7 +449,19 @@ func (ds *DataSource) setPreferredStoreType(hintInfo *tableHintInfo) { ds.preferStoreType = 0 return } - ds.preferStoreType |= preferTiKV + ds.preferStoreType |= preferTiFlash + hasTiFlashPath := false + for _, path := range ds.possibleAccessPaths { + if path.storeType == kv.TiFlash { + hasTiFlashPath = true + break + } + } + // TODO: For now, if there is a TiFlash hint for a table, we enforce a TiFlash path. But hint is just a suggestion + // for the planner. We can keep it since we need it to debug with PD and TiFlash. In future, this should be removed. + if !hasTiFlashPath { + ds.possibleAccessPaths = append(ds.possibleAccessPaths, &accessPath{isTablePath: true, storeType: kv.TiFlash}) + } } }