From fa5d70235ab2f9dbfb11065438de8069a0b186a0 Mon Sep 17 00:00:00 2001 From: Ruihao Chen Date: Wed, 30 Oct 2024 15:00:37 +0800 Subject: [PATCH 1/7] Fix case sensitive --- .../core/memtable_infoschema_extractor.go | 4 - .../core/memtable_predicate_extractor.go | 258 +++++++----------- 2 files changed, 103 insertions(+), 159 deletions(-) diff --git a/pkg/planner/core/memtable_infoschema_extractor.go b/pkg/planner/core/memtable_infoschema_extractor.go index 26079d2f553c6..d535c3cca9b17 100644 --- a/pkg/planner/core/memtable_infoschema_extractor.go +++ b/pkg/planner/core/memtable_infoschema_extractor.go @@ -262,10 +262,6 @@ func (e *InfoSchemaBaseExtractor) filter(colName string, val string) bool { } predVals, ok := e.ColPredicates[colName] if ok && len(predVals) > 0 { - fn, ok := e.pushedDownFuncs[colName] - if ok { - return !predVals.Exist(fn(val)) - } return !predVals.Exist(val) } // No need to filter records since no predicate for the column exists. diff --git a/pkg/planner/core/memtable_predicate_extractor.go b/pkg/planner/core/memtable_predicate_extractor.go index 77ef307c506e4..d8d138da921de 100644 --- a/pkg/planner/core/memtable_predicate_extractor.go +++ b/pkg/planner/core/memtable_predicate_extractor.go @@ -45,196 +45,147 @@ import ( "go.uber.org/zap" ) -// extractHelper contains some common utililty functions for all extractor. -// define an individual struct instead of a bunch of un-exported functions -// to avoid polluting the global scope of current package. -type extractHelper struct { - enableScalarPushDown bool - pushedDownFuncs map[string]func(string) string +// TODO(joechenrh): Maybe we can support other simple string functions later. +var supportedPushdownFuncs = map[string]struct{}{ + ast.Lower: {}, + ast.Upper: {}, } -func (extractHelper) extractColInConsExpr(ctx base.PlanContext, extractCols map[int64]*types.FieldName, expr *expression.ScalarFunction) (string, []types.Datum) { - args := expr.GetArgs() - col, isCol := args[0].(*expression.Column) - if !isCol { - return "", nil - } - name, found := extractCols[col.UniqueID] - if !found { - return "", nil +// extract const datum from expression +func extractConstant( + ctx base.PlanContext, + expr expression.Expression, +) (v types.Datum, found bool) { + found = false + + constant, ok := expr.(*expression.Constant) + if !ok || constant.DeferredExpr != nil { + return } - // All expressions in IN must be a constant - // SELECT * FROM t1 WHERE c IN ('1', '2') - results := make([]types.Datum, 0, len(args[1:])) - for _, arg := range args[1:] { - constant, ok := arg.(*expression.Constant) - if !ok || constant.DeferredExpr != nil { - return "", nil - } - v := constant.Value - if constant.ParamMarker != nil { - var err error - v, err = constant.ParamMarker.GetUserVar(ctx.GetExprCtx().GetEvalCtx()) - intest.AssertNoError(err, "fail to get param") - if err != nil { - logutil.BgLogger().Warn("fail to get param", zap.Error(err)) - return "", nil - } + v = constant.Value + if constant.ParamMarker != nil { + var err error + v, err = constant.ParamMarker.GetUserVar(ctx.GetExprCtx().GetEvalCtx()) + intest.AssertNoError(err, "fail to get param") + if err != nil { + logutil.BgLogger().Warn("fail to get param", zap.Error(err)) + return } - results = append(results, v) } - return name.ColName.L, results + + found = true + return } -func (helper *extractHelper) setColumnPushedDownFn( - colNameL string, - extractCols map[int64]*types.FieldName, - expr *expression.ScalarFunction, -) { - scalar := helper.extractColBinaryOpScalarFunc(extractCols, expr) - if scalar == nil { - return +func extractColumn( + expr expression.Expression, + supportPushdown bool, +) (*expression.Column, bool) { + if col, isCol := expr.(*expression.Column); isCol { + return col, true } - switch scalar.FuncName.L { - case ast.Lower: - helper.pushedDownFuncs = make(map[string]func(string) string) - helper.pushedDownFuncs[colNameL] = strings.ToLower - case ast.Upper: - helper.pushedDownFuncs = make(map[string]func(string) string) - helper.pushedDownFuncs[colNameL] = strings.ToUpper + if !supportPushdown { + return nil, false } -} -func (extractHelper) isPushDownSupported(fnNameL string) bool { - for _, s := range []string{ast.Lower, ast.Upper} { - if fnNameL == s { - return true - } + scalar, isScalar := expr.(*expression.ScalarFunction) + if !isScalar || scalar == nil { + return nil, false } - return false -} -// extractColBinaryOpScalarFunc extract the scalar function from a binary operation. For example, -// `eq(lower(col), "constant")` returns `lower`. -func (extractHelper) extractColBinaryOpScalarFunc( - extractCols map[int64]*types.FieldName, - expr *expression.ScalarFunction, -) (sf *expression.ScalarFunction) { - args := expr.GetArgs() - var constIdx int - // c = 'rhs' - // 'lhs' = c - for i := 0; i < 2; i++ { - _, isConst := args[i].(*expression.Constant) - if isConst { - constIdx = i - break - } - } - scalar, isScalar := args[1-constIdx].(*expression.ScalarFunction) - if !isScalar { - return nil - } - args = scalar.GetArgs() + args := scalar.GetArgs() if len(args) != 1 { - return nil + return nil, false } + col, isCol := args[0].(*expression.Column) if !isCol { - return nil + return nil, false } - _, found := extractCols[col.UniqueID] - if !found { - return nil + if _, ok := supportedPushdownFuncs[scalar.FuncName.L]; !ok { + return nil, false } - return scalar + return col, true } -func (helper *extractHelper) tryToFindInnerColAndIdx(args []expression.Expression) (innerCol *expression.Column, colIdx int) { - if !helper.enableScalarPushDown { - return nil, -1 +// extractHelper contains some common utililty functions for all extractor. +// define an individual struct instead of a bunch of un-exported functions +// to avoid polluting the global scope of current package. +type extractHelper struct { + enableScalarPushDown bool +} + +// extract column name and const data in IN expression +func (helper *extractHelper) extractFromInConsExpr( + ctx base.PlanContext, + extractCols map[int64]*types.FieldName, + expr *expression.ScalarFunction, +) (string, []types.Datum) { + args := expr.GetArgs() + + col, ok := extractColumn(args[0], helper.enableScalarPushDown) + if !ok { + return "", nil } - var scalar *expression.ScalarFunction - for i := 0; i < 2; i++ { - var isScalar bool - scalar, isScalar = args[i].(*expression.ScalarFunction) - if isScalar { - colIdx = i - break - } + name, found := extractCols[col.UniqueID] + if !found { + return "", nil } - if scalar != nil { - args := scalar.GetArgs() - if len(args) != 1 { - return nil, -1 - } - col, isCol := args[0].(*expression.Column) - if !isCol { - return nil, -1 - } - if !helper.isPushDownSupported(scalar.FuncName.L) { - return nil, -1 + // All expressions in IN must be a constant + // SELECT * FROM t1 WHERE c IN ('1', '2') + results := make([]types.Datum, 0, len(args[1:])) + for _, arg := range args[1:] { + v, ok := extractConstant(ctx, arg) + if !ok { + return "", nil } - return col, colIdx + results = append(results, v) } - return nil, -1 + return name.ColName.L, results } -func (helper *extractHelper) extractColBinaryOpConsExpr( +// extract the AND expression, e.g: +// SELECT * FROM t1 WHERE c='rhs' +// SELECT * FROM t1 WHERE 'lhs'=c +func (helper *extractHelper) extractFromBinaryOpConsExpr( ctx base.PlanContext, extractCols map[int64]*types.FieldName, expr *expression.ScalarFunction, ) (string, []types.Datum) { + var ( + scalarIdx int = 1 + ok bool + col *expression.Column + v types.Datum + ) + args := expr.GetArgs() - var col *expression.Column - var colIdx int - // c = 'rhs' - // 'lhs' = c - for i := 0; i < 2; i++ { - var isCol bool - col, isCol = args[i].(*expression.Column) - if isCol { - colIdx = i - break + if col, ok = extractColumn(args[0], helper.enableScalarPushDown); !ok { + scalarIdx = 0 + if col, ok = extractColumn(args[1], helper.enableScalarPushDown); !ok { + return "", nil } } - innerCol, innerColIdx := helper.tryToFindInnerColAndIdx(args) - if innerCol != nil { - col, colIdx = innerCol, innerColIdx - } - if col == nil { - return "", nil - } - name, found := extractCols[col.UniqueID] if !found { return "", nil } - // The `lhs/rhs` of EQ expression must be a constant - // SELECT * FROM t1 WHERE c='rhs' - // SELECT * FROM t1 WHERE 'lhs'=c - constant, ok := args[1-colIdx].(*expression.Constant) - if !ok || constant.DeferredExpr != nil { + v, ok = extractConstant(ctx, args[scalarIdx]) + if !ok { return "", nil } - v := constant.Value - if constant.ParamMarker != nil { - var err error - v, err = constant.ParamMarker.GetUserVar(ctx.GetExprCtx().GetEvalCtx()) - intest.AssertNoError(err, "fail to get param") - if err != nil { - logutil.BgLogger().Warn("fail to get param", zap.Error(err)) - return "", nil - } - } return name.ColName.L, []types.Datum{v} } // extract the OR expression, e.g: // SELECT * FROM t1 WHERE c1='a' OR c1='b' OR c1='c' -func (helper *extractHelper) extractColOrExpr(ctx base.PlanContext, extractCols map[int64]*types.FieldName, expr *expression.ScalarFunction) (string, []types.Datum) { +func (helper *extractHelper) extractFromOrExpr( + ctx base.PlanContext, + extractCols map[int64]*types.FieldName, + expr *expression.ScalarFunction, +) (string, []types.Datum) { args := expr.GetArgs() lhs, ok := args[0].(*expression.ScalarFunction) if !ok { @@ -248,11 +199,11 @@ func (helper *extractHelper) extractColOrExpr(ctx base.PlanContext, extractCols var extract = func(extractCols map[int64]*types.FieldName, fn *expression.ScalarFunction) (string, []types.Datum) { switch helper.getStringFunctionName(fn) { case ast.EQ: - return helper.extractColBinaryOpConsExpr(ctx, extractCols, fn) + return helper.extractFromBinaryOpConsExpr(ctx, extractCols, fn) case ast.LogicOr: - return helper.extractColOrExpr(ctx, extractCols, fn) + return helper.extractFromOrExpr(ctx, extractCols, fn) case ast.In: - return helper.extractColInConsExpr(ctx, extractCols, fn) + return helper.extractFromInConsExpr(ctx, extractCols, fn) default: return "", nil } @@ -320,15 +271,12 @@ func (helper *extractHelper) extractCol( switch helper.getStringFunctionName(fn) { case ast.EQ: helper.enableScalarPushDown = true - colName, datums = helper.extractColBinaryOpConsExpr(ctx, extractCols, fn) - if colName == extractColName { - helper.setColumnPushedDownFn(colName, extractCols, fn) - } + colName, datums = helper.extractFromBinaryOpConsExpr(ctx, extractCols, fn) helper.enableScalarPushDown = false case ast.In: - colName, datums = helper.extractColInConsExpr(ctx, extractCols, fn) + colName, datums = helper.extractFromInConsExpr(ctx, extractCols, fn) case ast.LogicOr: - colName, datums = helper.extractColOrExpr(ctx, extractCols, fn) + colName, datums = helper.extractFromOrExpr(ctx, extractCols, fn) } if colName == extractColName { result = helper.merge(result, datums, valueToLower) @@ -445,7 +393,7 @@ func (helper extractHelper) extractLikePattern( var datums []types.Datum switch fn.FuncName.L { case ast.EQ, ast.Like, ast.Ilike, ast.Regexp, ast.RegexpLike: - colName, datums = helper.extractColBinaryOpConsExpr(ctx, extractCols, fn) + colName, datums = helper.extractFromBinaryOpConsExpr(ctx, extractCols, fn) } if colName != extractColName { return false, "" @@ -551,7 +499,7 @@ func (helper extractHelper) extractTimeRange( fnName := helper.getTimeFunctionName(fn) switch fnName { case ast.GT, ast.GE, ast.LT, ast.LE, ast.EQ: - colName, datums = helper.extractColBinaryOpConsExpr(ctx, extractCols, fn) + colName, datums = helper.extractFromBinaryOpConsExpr(ctx, extractCols, fn) } if colName == extractColName { From b586b97d2de56b32a4482773c0267925e2f88ff7 Mon Sep 17 00:00:00 2001 From: Ruihao Chen Date: Wed, 30 Oct 2024 16:48:20 +0800 Subject: [PATCH 2/7] Fix case sensitive --- .../core/memtable_predicate_extractor.go | 63 ++++++++++++++++--- 1 file changed, 56 insertions(+), 7 deletions(-) diff --git a/pkg/planner/core/memtable_predicate_extractor.go b/pkg/planner/core/memtable_predicate_extractor.go index d8d138da921de..f63f77f3915e4 100644 --- a/pkg/planner/core/memtable_predicate_extractor.go +++ b/pkg/planner/core/memtable_predicate_extractor.go @@ -51,6 +51,51 @@ var supportedPushdownFuncs = map[string]struct{}{ ast.Upper: {}, } +func (helper *extractHelper) rewriteExpression( + expr expression.Expression, + colName string, + colIDMapping map[int64]*types.FieldName, +) expression.Expression { + e, ok := expr.(*expression.ScalarFunction) + if !ok { + return expr + } + + // switch helper.getStringFunctionName(e) { + // case ast.EQ: + // return helper.extractFromBinaryOpConsExpr(ctx, extractCols, fn) + // case ast.LogicOr: + // return helper.extractFromOrExpr(ctx, extractCols, fn) + // case ast.In: + // return helper.extractFromInConsExpr(ctx, extractCols, fn) + // default: + // return "", nil + // } + + if _, ok := supportedPushdownFuncs[e.FuncName.L]; ok { + args := e.GetArgs() + if len(args) != 1 { + return e + } + + newArg := helper.rewriteExpression(args[0], colName, colIDMapping) + if col, ok := newArg.(*expression.Column); ok { + if name, ok := colIDMapping[col.ID]; ok && name.ColName.L == colName { + return col + } + } + args[0] = newArg + return e + } + + args := e.GetArgs() + for i, arg := range args { + args[i] = helper.rewriteExpression(arg, colName, colIDMapping) + } + + return e +} + // extract const datum from expression func extractConstant( ctx base.PlanContext, @@ -246,7 +291,7 @@ func (helper *extractHelper) extractCol( names []*types.FieldName, predicates []expression.Expression, extractColName string, - valueToLower bool, + caseInsensitive bool, ) ( remained []expression.Expression, skipRequest bool, @@ -254,13 +299,17 @@ func (helper *extractHelper) extractCol( ) { remained = make([]expression.Expression, 0, len(predicates)) result = set.NewStringSet() - extractCols := helper.findColumn(schema, names, extractColName) - if len(extractCols) == 0 { + extractColIDMapping := helper.findColumn(schema, names, extractColName) + if len(extractColIDMapping) == 0 { return predicates, false, result } // We should use INTERSECTION of sets because of the predicates is CNF array for _, expr := range predicates { + if caseInsensitive { + expr = helper.rewriteExpression(expr, extractColName, extractColIDMapping) + } + fn, ok := expr.(*expression.ScalarFunction) if !ok { remained = append(remained, expr) @@ -271,15 +320,15 @@ func (helper *extractHelper) extractCol( switch helper.getStringFunctionName(fn) { case ast.EQ: helper.enableScalarPushDown = true - colName, datums = helper.extractFromBinaryOpConsExpr(ctx, extractCols, fn) + colName, datums = helper.extractFromBinaryOpConsExpr(ctx, extractColIDMapping, fn) helper.enableScalarPushDown = false case ast.In: - colName, datums = helper.extractFromInConsExpr(ctx, extractCols, fn) + colName, datums = helper.extractFromInConsExpr(ctx, extractColIDMapping, fn) case ast.LogicOr: - colName, datums = helper.extractFromOrExpr(ctx, extractCols, fn) + colName, datums = helper.extractFromOrExpr(ctx, extractColIDMapping, fn) } if colName == extractColName { - result = helper.merge(result, datums, valueToLower) + result = helper.merge(result, datums, caseInsensitive) skipRequest = len(result) == 0 } else { remained = append(remained, expr) From 54c9b8cd7a84aa4123d06eb56c5574f1b5f38d9c Mon Sep 17 00:00:00 2001 From: Ruihao Chen Date: Thu, 31 Oct 2024 11:03:39 +0800 Subject: [PATCH 3/7] Make query without predicate pushdown case insensitive --- pkg/executor/infoschema_reader_test.go | 45 ++++++++++++ .../core/memtable_infoschema_extractor.go | 7 +- .../core/memtable_predicate_extractor.go | 72 +++++++++---------- 3 files changed, 85 insertions(+), 39 deletions(-) diff --git a/pkg/executor/infoschema_reader_test.go b/pkg/executor/infoschema_reader_test.go index ef73cc970c912..7b0de65016b59 100644 --- a/pkg/executor/infoschema_reader_test.go +++ b/pkg/executor/infoschema_reader_test.go @@ -1066,3 +1066,48 @@ func TestInfoschemaTablesSpecialOptimizationCovered(t *testing.T) { require.Equal(t, testCase.expect, covered, testCase.sql) } } + +func TestCaseInsensitiveInfoSchemaRead(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + + tk.MustExec("create database Test1") + tk.MustExec("create table Test1.Table1(Col1 int)") + + ops := []string{"=", "!=", ">", ">=", "<", "<="} + + testCases := []struct { + colName string + filter string + expected []int + }{ + { + colName: "table_name", + filter: `"Table1"`, + expected: []int{1, 822, 561, 562, 261, 262}, + }, + { + colName: "table_schema", + filter: `"Test1"`, + expected: []int{1, 822, 0, 1, 822, 823}, + }, + } + + sqlTemplates := []string{ + "select count(*) from information_schema.tables where %s(%s) %s %s", + `select count(*) from information_schema.tables where %s(%s) %s %s and table_type != ""`, // disable count(*) optimization + } + for _, tc := range testCases { + for i, op := range ops { + for _, st := range sqlTemplates { + for _, stringFunc := range []string{"lower", "upper", ""} { + res := tk.MustQuery(fmt.Sprintf(st, stringFunc, tc.colName, op, tc.filter)) + s := res.Rows()[0][0].(string) + actual, err := strconv.Atoi(s) + require.NoError(t, err) + require.Equal(t, tc.expected[i], actual) + } + } + } + } +} diff --git a/pkg/planner/core/memtable_infoschema_extractor.go b/pkg/planner/core/memtable_infoschema_extractor.go index d535c3cca9b17..ea4a793d97de7 100644 --- a/pkg/planner/core/memtable_infoschema_extractor.go +++ b/pkg/planner/core/memtable_infoschema_extractor.go @@ -212,7 +212,12 @@ func (e *InfoSchemaBaseExtractor) Extract( e.LikePatterns[colName] = likePatterns e.colsRegexp[colName] = regexp } - return remained + rewrited := make([]expression.Expression, 0, len(remained)) + for _, expr := range remained { + newExpr := e.convertToLowerInExpression(ctx, expr) + rewrited = append(rewrited, newExpr) + } + return rewrited } // ExplainInfo implements base.MemTablePredicateExtractor interface. diff --git a/pkg/planner/core/memtable_predicate_extractor.go b/pkg/planner/core/memtable_predicate_extractor.go index f63f77f3915e4..03c40f03fc46d 100644 --- a/pkg/planner/core/memtable_predicate_extractor.go +++ b/pkg/planner/core/memtable_predicate_extractor.go @@ -45,55 +45,55 @@ import ( "go.uber.org/zap" ) -// TODO(joechenrh): Maybe we can support other simple string functions later. var supportedPushdownFuncs = map[string]struct{}{ ast.Lower: {}, ast.Upper: {}, } -func (helper *extractHelper) rewriteExpression( +// convertToLowerInExpression does the following works: +// 1. Add lower function to all columns. +// 2. Remove all upper functions. +// 3. Convert all string constant to lower case. +// It's to make upper Selection Operator case insensitive. +func (helper *extractHelper) convertToLowerInExpression( + ctx base.PlanContext, expr expression.Expression, - colName string, - colIDMapping map[int64]*types.FieldName, ) expression.Expression { - e, ok := expr.(*expression.ScalarFunction) - if !ok { - return expr - } - - // switch helper.getStringFunctionName(e) { - // case ast.EQ: - // return helper.extractFromBinaryOpConsExpr(ctx, extractCols, fn) - // case ast.LogicOr: - // return helper.extractFromOrExpr(ctx, extractCols, fn) - // case ast.In: - // return helper.extractFromInConsExpr(ctx, extractCols, fn) - // default: - // return "", nil - // } - - if _, ok := supportedPushdownFuncs[e.FuncName.L]; ok { + switch e := expr.(type) { + case *expression.ScalarFunction: + fn := helper.getStringFunctionName(e) args := e.GetArgs() - if len(args) != 1 { + + switch fn { + case ast.Lower: + return e + case ast.Upper: + if len(args) != 1 { + return e + } + return helper.convertToLowerInExpression(ctx, args[0]) + default: + for i, arg := range args { + args[i] = helper.convertToLowerInExpression(ctx, arg) + } return e } - - newArg := helper.rewriteExpression(args[0], colName, colIDMapping) - if col, ok := newArg.(*expression.Column); ok { - if name, ok := colIDMapping[col.ID]; ok && name.ColName.L == colName { - return col + case *expression.Column: + if e.RetType.EvalType() == types.ETString { + if newArg, err := expression.NewFunction(ctx.GetExprCtx(), ast.Lower, e.RetType, e); err == nil { + return newArg } } - args[0] = newArg + return e + case *expression.Constant: + v := e.Value + if v.Kind() == types.KindString { + e.Value.SetString(strings.ToLower(v.GetString()), v.Collation()) + } return e } - args := e.GetArgs() - for i, arg := range args { - args[i] = helper.rewriteExpression(arg, colName, colIDMapping) - } - - return e + return expr } // extract const datum from expression @@ -306,10 +306,6 @@ func (helper *extractHelper) extractCol( // We should use INTERSECTION of sets because of the predicates is CNF array for _, expr := range predicates { - if caseInsensitive { - expr = helper.rewriteExpression(expr, extractColName, extractColIDMapping) - } - fn, ok := expr.(*expression.ScalarFunction) if !ok { remained = append(remained, expr) From e77c92a3f1f6fd54414675f088ce82e4eac9a961 Mon Sep 17 00:00:00 2001 From: Ruihao Chen Date: Thu, 31 Oct 2024 11:20:27 +0800 Subject: [PATCH 4/7] fix build --- pkg/planner/core/memtable_predicate_extractor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/planner/core/memtable_predicate_extractor.go b/pkg/planner/core/memtable_predicate_extractor.go index 03c40f03fc46d..c686e999adfb0 100644 --- a/pkg/planner/core/memtable_predicate_extractor.go +++ b/pkg/planner/core/memtable_predicate_extractor.go @@ -198,7 +198,7 @@ func (helper *extractHelper) extractFromBinaryOpConsExpr( expr *expression.ScalarFunction, ) (string, []types.Datum) { var ( - scalarIdx int = 1 + scalarIdx = 1 ok bool col *expression.Column v types.Datum From 93519b24064990d70cf6f542ba12b7421c413d1f Mon Sep 17 00:00:00 2001 From: Ruihao Chen Date: Thu, 31 Oct 2024 14:08:30 +0800 Subject: [PATCH 5/7] Fix integrationtest --- tests/integrationtest/r/infoschema/infoschema.result | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integrationtest/r/infoschema/infoschema.result b/tests/integrationtest/r/infoschema/infoschema.result index dd8c024b952e9..8ff0bd1b4490e 100644 --- a/tests/integrationtest/r/infoschema/infoschema.result +++ b/tests/integrationtest/r/infoschema/infoschema.result @@ -112,7 +112,7 @@ InnoDB NONCLUSTERED explain select engine, tidb_pk_type from information_schema.tables where (table_name ='t4' or upper(table_name) = 'T5') and table_schema = 'infoschema__infoschema'; id estRows task access object operator info Projection_4 8000.00 root Column#5, Column#24 -└─Selection_5 8000.00 root or(eq(Column#3, "t4"), eq(upper(Column#3), "T5")) +└─Selection_5 8000.00 root or(eq(lower(Column#3), "t4"), eq(lower(Column#3), "t5")) └─MemTableScan_6 10000.00 root table:TABLES table_schema:["infoschema__infoschema"] select engine, tidb_pk_type from information_schema.tables where lower(table_name) = 't5' and upper(table_schema) = 'INFOSCHEMA__INFOSCHEMA'; engine tidb_pk_type @@ -120,7 +120,7 @@ InnoDB NONCLUSTERED explain select engine, tidb_pk_type from information_schema.tables where (table_name ='t4' or lower(table_name) = 't5') and upper(table_schema) = 'INFOSCHEMA__INFOSCHEMA'; id estRows task access object operator info Projection_4 8000.00 root Column#5, Column#24 -└─Selection_5 8000.00 root or(eq(Column#3, "t4"), eq(lower(Column#3), "t5")) +└─Selection_5 8000.00 root or(eq(lower(Column#3), "t4"), eq(lower(Column#3), "t5")) └─MemTableScan_6 10000.00 root table:TABLES table_schema:["infoschema__infoschema"] select engine, tidb_pk_type from information_schema.tables where (table_name ='t4' or table_name = 't5') and table_schema = 'infoschema__infoschema'; engine tidb_pk_type From d9d33b7730f99d562ad35176dec94322e8e2e22b Mon Sep 17 00:00:00 2001 From: Ruihao Chen Date: Fri, 1 Nov 2024 10:19:49 +0800 Subject: [PATCH 6/7] Fix integrationtest --- tests/integrationtest/r/executor/explainfor.result | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integrationtest/r/executor/explainfor.result b/tests/integrationtest/r/executor/explainfor.result index 2233e07dc4ffe..dec600e4b1fe6 100644 --- a/tests/integrationtest/r/executor/explainfor.result +++ b/tests/integrationtest/r/executor/explainfor.result @@ -891,6 +891,6 @@ desc format='brief' SELECT * FROM information_schema.referential_constraints rc id estRows task access object operator info Projection 8000.00 root Column#2, Column#3, Column#1, Column#4, Column#5, Column#6, Column#7, Column#8, Column#9, Column#10, Column#11, Column#12, Column#15, Column#16, Column#17, Column#18, Column#19, Column#20, Column#21, Column#22, Column#23 └─HashJoin 8000.00 root inner join, equal:[eq(Column#3, Column#14)] - ├─Selection(Build) 8000.00 root ne(Column#23, "t") + ├─Selection(Build) 8000.00 root ne(lower(Column#23), "t") │ └─MemTableScan 10000.00 root table:KEY_COLUMN_USAGE constraint_schema:["plan_cache"], table_name:["t"], table_schema:["plan_cache"] └─MemTableScan(Probe) 10000.00 root table:REFERENTIAL_CONSTRAINTS constraint_schema:["plan_cache"], table_name:["t"] From 8c5cfa44bef4c3603f276e5c4c694fbe1a284a03 Mon Sep 17 00:00:00 2001 From: Ruihao Chen Date: Mon, 18 Nov 2024 11:47:43 +0800 Subject: [PATCH 7/7] Address comments --- .../core/memtable_infoschema_extractor.go | 9 +++-- .../core/memtable_predicate_extractor.go | 34 +++++++++---------- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/pkg/planner/core/memtable_infoschema_extractor.go b/pkg/planner/core/memtable_infoschema_extractor.go index ea4a793d97de7..0683241d2e421 100644 --- a/pkg/planner/core/memtable_infoschema_extractor.go +++ b/pkg/planner/core/memtable_infoschema_extractor.go @@ -212,12 +212,11 @@ func (e *InfoSchemaBaseExtractor) Extract( e.LikePatterns[colName] = likePatterns e.colsRegexp[colName] = regexp } - rewrited := make([]expression.Expression, 0, len(remained)) - for _, expr := range remained { - newExpr := e.convertToLowerInExpression(ctx, expr) - rewrited = append(rewrited, newExpr) + + for i, expr := range remained { + remained[i] = e.convertToLowerInExpression(ctx, expr) } - return rewrited + return } // ExplainInfo implements base.MemTablePredicateExtractor interface. diff --git a/pkg/planner/core/memtable_predicate_extractor.go b/pkg/planner/core/memtable_predicate_extractor.go index c686e999adfb0..45defcf3fe7f2 100644 --- a/pkg/planner/core/memtable_predicate_extractor.go +++ b/pkg/planner/core/memtable_predicate_extractor.go @@ -125,32 +125,32 @@ func extractConstant( func extractColumn( expr expression.Expression, supportPushdown bool, -) (*expression.Column, bool) { +) *expression.Column { if col, isCol := expr.(*expression.Column); isCol { - return col, true + return col } if !supportPushdown { - return nil, false + return nil } scalar, isScalar := expr.(*expression.ScalarFunction) if !isScalar || scalar == nil { - return nil, false + return nil } args := scalar.GetArgs() if len(args) != 1 { - return nil, false + return nil } col, isCol := args[0].(*expression.Column) if !isCol { - return nil, false + return nil } if _, ok := supportedPushdownFuncs[scalar.FuncName.L]; !ok { - return nil, false + return nil } - return col, true + return col } // extractHelper contains some common utililty functions for all extractor. @@ -168,8 +168,8 @@ func (helper *extractHelper) extractFromInConsExpr( ) (string, []types.Datum) { args := expr.GetArgs() - col, ok := extractColumn(args[0], helper.enableScalarPushDown) - if !ok { + col := extractColumn(args[0], helper.enableScalarPushDown) + if col == nil { return "", nil } name, found := extractCols[col.UniqueID] @@ -198,16 +198,14 @@ func (helper *extractHelper) extractFromBinaryOpConsExpr( expr *expression.ScalarFunction, ) (string, []types.Datum) { var ( - scalarIdx = 1 - ok bool - col *expression.Column - v types.Datum + constIdx = 1 + col *expression.Column ) args := expr.GetArgs() - if col, ok = extractColumn(args[0], helper.enableScalarPushDown); !ok { - scalarIdx = 0 - if col, ok = extractColumn(args[1], helper.enableScalarPushDown); !ok { + if col = extractColumn(args[0], helper.enableScalarPushDown); col == nil { + constIdx = 0 + if col = extractColumn(args[1], helper.enableScalarPushDown); col == nil { return "", nil } } @@ -217,7 +215,7 @@ func (helper *extractHelper) extractFromBinaryOpConsExpr( return "", nil } - v, ok = extractConstant(ctx, args[scalarIdx]) + v, ok := extractConstant(ctx, args[constIdx]) if !ok { return "", nil }