Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

planner: add missing CanExprsPushDown checks when generating IndexMerge path for or (#41361) #41396

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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