Skip to content

Commit

Permalink
planner: add missing CanExprsPushDown checks when generating IndexM…
Browse files Browse the repository at this point in the history
…erge path for `or` (#41361) (#41396)

close #41273, close #41293
  • Loading branch information
ti-chi-bot authored Mar 29, 2023
1 parent d8b1536 commit aac228a
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 8 deletions.
25 changes: 25 additions & 0 deletions planner/core/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6713,3 +6713,28 @@ func TestAutoIncrementCheckWithCheckConstraint(t *testing.T) {
KEY idx_autoinc_id (id)
)`)
}

// https://github.com/pingcap/tidb/issues/41273
func TestIssue41273(t *testing.T) {
store, clean := testkit.CreateMockStore(t)
defer clean()
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec(`CREATE TABLE t (
a set('nwbk','r5','1ad3u','van','ir1z','y','9m','f1','z','e6yd','wfev') NOT NULL DEFAULT 'ir1z,f1,e6yd',
b enum('soo2','4s4j','qi9om','8ue','i71o','qon','3','3feh','6o1i','5yebx','d') NOT NULL DEFAULT '8ue',
c varchar(66) DEFAULT '13mdezixgcn',
PRIMARY KEY (a,b) /*T![clustered_index] CLUSTERED */,
UNIQUE KEY ib(b),
KEY ia(a)
)ENGINE=InnoDB DEFAULT CHARSET=ascii COLLATE=ascii_bin;`)
tk.MustExec("INSERT INTO t VALUES('ir1z,f1,e6yd','i71o','13mdezixgcn'),('ir1z,f1,e6yd','d','13mdezixgcn'),('nwbk','8ue','13mdezixgcn');")
expectedRes := []string{"ir1z,f1,e6yd d 13mdezixgcn", "ir1z,f1,e6yd i71o 13mdezixgcn", "nwbk 8ue 13mdezixgcn"}
tk.MustQuery("select * from t where a between 'e6yd' and 'z' or b <> '8ue';").Sort().Check(testkit.Rows(expectedRes...))
tk.MustQuery("select /*+ use_index_merge(t) */ * from t where a between 'e6yd' and 'z' or b <> '8ue';").Sort().Check(testkit.Rows(expectedRes...))
// For now tidb doesn't support push set type to TiKV, and column a is a set type, so we shouldn't generate a IndexMerge path.
require.False(t,
tk.HasPlan("select /*+ use_index_merge(t) */ * from t where a between 'e6yd' and 'z' or b <> '8ue'",
"IndexMerge"),
)
}
37 changes: 29 additions & 8 deletions planner/core/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -557,11 +557,28 @@ func (ds *DataSource) generateIndexMergeOrPaths(filters []expression.Expression)
if !ok || sf.FuncName.L != ast.LogicOr {
continue
}
// shouldKeepCurrentFilter means the partial paths can't cover the current filter completely, so we must add
// the current filter into a Selection after partial paths.
shouldKeepCurrentFilter := false
var partialPaths = make([]*util.AccessPath, 0, usedIndexCount)
dnfItems := expression.FlattenDNFConditions(sf)
for _, item := range dnfItems {
cnfItems := expression.SplitCNFItems(item)
itemPaths := ds.accessPathsForConds(cnfItems, usedIndexCount)

pushedDownCNFItems := make([]expression.Expression, 0, len(cnfItems))
for _, cnfItem := range cnfItems {
if expression.CanExprsPushDown(ds.ctx.GetSessionVars().StmtCtx,
[]expression.Expression{cnfItem},
ds.ctx.GetClient(),
kv.TiKV,
) {
pushedDownCNFItems = append(pushedDownCNFItems, cnfItem)
} else {
shouldKeepCurrentFilter = true
}
}

itemPaths := ds.accessPathsForConds(pushedDownCNFItems, usedIndexCount)
if len(itemPaths) == 0 {
partialPaths = nil
break
Expand All @@ -588,7 +605,7 @@ func (ds *DataSource) generateIndexMergeOrPaths(filters []expression.Expression)
continue
}
if len(partialPaths) > 1 {
possiblePath := ds.buildIndexMergeOrPath(filters, partialPaths, i)
possiblePath := ds.buildIndexMergeOrPath(filters, partialPaths, i, shouldKeepCurrentFilter)
if possiblePath == nil {
return nil
}
Expand Down Expand Up @@ -726,28 +743,32 @@ func (ds *DataSource) buildIndexMergePartialPath(indexAccessPaths []*util.Access
}

// buildIndexMergeOrPath generates one possible IndexMergePath.
func (ds *DataSource) buildIndexMergeOrPath(filters []expression.Expression, partialPaths []*util.AccessPath, current int) *util.AccessPath {
func (ds *DataSource) buildIndexMergeOrPath(
filters []expression.Expression,
partialPaths []*util.AccessPath,
current int,
shouldKeepCurrentFilter bool,
) *util.AccessPath {
indexMergePath := &util.AccessPath{PartialIndexPaths: partialPaths}
indexMergePath.TableFilters = append(indexMergePath.TableFilters, filters[:current]...)
indexMergePath.TableFilters = append(indexMergePath.TableFilters, filters[current+1:]...)
var addCurrentFilter bool
for _, path := range partialPaths {
// If any partial path contains table filters, we need to keep the whole DNF filter in the Selection.
if len(path.TableFilters) > 0 {
addCurrentFilter = true
shouldKeepCurrentFilter = true
}
// If any partial path's index filter cannot be pushed to TiKV, we should keep the whole DNF filter.
if len(path.IndexFilters) != 0 && !expression.CanExprsPushDown(ds.ctx.GetSessionVars().StmtCtx, path.IndexFilters, ds.ctx.GetClient(), kv.TiKV) {
addCurrentFilter = true
shouldKeepCurrentFilter = true
// Clear IndexFilter, the whole filter will be put in indexMergePath.TableFilters.
path.IndexFilters = nil
}
if len(path.TableFilters) != 0 && !expression.CanExprsPushDown(ds.ctx.GetSessionVars().StmtCtx, path.TableFilters, ds.ctx.GetClient(), kv.TiKV) {
addCurrentFilter = true
shouldKeepCurrentFilter = true
path.TableFilters = nil
}
}
if addCurrentFilter {
if shouldKeepCurrentFilter {
indexMergePath.TableFilters = append(indexMergePath.TableFilters, filters[current])
}
return indexMergePath
Expand Down

0 comments on commit aac228a

Please sign in to comment.