From c5e518876c953da9bc5487d056900a00196b8a73 Mon Sep 17 00:00:00 2001 From: yibin Date: Wed, 16 Aug 2023 10:01:05 +0800 Subject: [PATCH 1/4] Just comments change Signed-off-by: yibin --- expression/builtin_compare.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/expression/builtin_compare.go b/expression/builtin_compare.go index 1aa1f65d8bed5..3a55ce3d9fd23 100644 --- a/expression/builtin_compare.go +++ b/expression/builtin_compare.go @@ -1597,7 +1597,7 @@ func allowCmpArgsRefining4PlanCache(ctx sessionctx.Context, args []Expression) ( return false } -// refineArgs will rewrite the arguments if the compare expression is `int column non-int constant` or +// refineArg will rewrite the arguments if the compare expression is `int column non-int constant` or // `non-int constant int column`. E.g., `a < 1.1` will be rewritten to `a < 2`. It also handles comparing year type // with int constant if the int constant falls into a sensible year representation. // This refine operation depends on the values of these args, but these values can change when using plan-cache. From cf453fcce5a011d335f912eeae5e9c3981048505 Mon Sep 17 00:00:00 2001 From: yibin Date: Wed, 16 Aug 2023 10:40:54 +0800 Subject: [PATCH 2/4] Submit actual cherry pick changes Signed-off-by: yibin --- executor/executor_test.go | 42 ++++++++++++++++++++ expression/builtin_compare.go | 73 ++++++++++++++++++++++++++++++----- 2 files changed, 105 insertions(+), 10 deletions(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index de43d423c39a6..f24de4bdbade5 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -6511,3 +6511,45 @@ func TestProcessInfoOfSubQuery(t *testing.T) { tk2.MustQuery("select 1 from information_schema.processlist where TxnStart != '' and info like 'select%sleep% from t%'").Check(testkit.Rows("1")) wg.Wait() } + +func TestCompareIssue38361(t *testing.T) { + store := testkit.CreateMockStore(t) + + tk := testkit.NewTestKit(t, store) + tk.MustExec("drop database if exists TEST1") + tk.MustExec("create database TEST1") + tk.MustExec("use TEST1") + tk.MustExec("create table t(a datetime, b bigint, c bigint)") + tk.MustExec("insert into t values(cast('2023-08-09 00:00:00' as datetime), 20230809, 20231310)") + + tk.MustQuery("select a > 20230809 from t").Check(testkit.Rows("0")) + tk.MustQuery("select a = 20230809 from t").Check(testkit.Rows("1")) + tk.MustQuery("select a < 20230810 from t").Check(testkit.Rows("1")) + //// 20231310 can't be converted to valid datetime, thus should be compared using real date type,and datetime will be + //// converted to something like 'YYYYMMDDHHMMSS', bigger than 20231310 + tk.MustQuery("select a < 20231310 from t").Check(testkit.Rows("0")) + tk.MustQuery("select 20230809 < a from t").Check(testkit.Rows("0")) + tk.MustQuery("select 20230809 = a from t").Check(testkit.Rows("1")) + tk.MustQuery("select 20230810 > a from t").Check(testkit.Rows("1")) + tk.MustQuery("select 20231310 > a from t").Check(testkit.Rows("0")) + + //// constant datetime cmp numeric constant should be compared as real data type + tk.MustQuery("select cast('2023-08-09 00:00:00' as datetime) > 20230809 from t").Check(testkit.Rows("1")) + tk.MustQuery("select cast('2023-08-09 00:00:00' as datetime) = 20230809 from t").Check(testkit.Rows("0")) + tk.MustQuery("select cast('2023-08-09 00:00:00' as datetime) < 20230810 from t").Check(testkit.Rows("0")) + tk.MustQuery("select cast('2023-08-09 00:00:00' as datetime) < 20231310 from t").Check(testkit.Rows("0")) + tk.MustQuery("select 20230809 < cast('2023-08-09 00:00:00' as datetime) from t").Check(testkit.Rows("1")) + tk.MustQuery("select 20230809 = cast('2023-08-09 00:00:00' as datetime) from t").Check(testkit.Rows("0")) + tk.MustQuery("select 20230810 > cast('2023-08-09 00:00:00' as datetime) from t").Check(testkit.Rows("0")) + tk.MustQuery("select 20231310 > cast('2023-08-09 00:00:00' as datetime) from t").Check(testkit.Rows("0")) + + //// datetime column cmp numeric column should be compared as real data type + tk.MustQuery("select a > b from t").Check(testkit.Rows("1")) + tk.MustQuery("select a = b from t").Check(testkit.Rows("0")) + tk.MustQuery("select a < b + 1 from t").Check(testkit.Rows("0")) + tk.MustQuery("select a < c from t").Check(testkit.Rows("0")) + tk.MustQuery("select b < a from t").Check(testkit.Rows("1")) + tk.MustQuery("select b = a from t").Check(testkit.Rows("0")) + tk.MustQuery("select b > a from t").Check(testkit.Rows("0")) + tk.MustQuery("select c > a from t").Check(testkit.Rows("0")) +} diff --git a/expression/builtin_compare.go b/expression/builtin_compare.go index 3a55ce3d9fd23..0f894ee728f3d 100644 --- a/expression/builtin_compare.go +++ b/expression/builtin_compare.go @@ -1556,6 +1556,11 @@ func RefineComparedConstant(ctx sessionctx.Context, targetFieldType types.FieldT return con, false } +func matchRefineRule3Pattern(conEvalType types.EvalType, exprType *types.FieldType) bool { + return (exprType.GetType() == mysql.TypeTimestamp || exprType.GetType() == mysql.TypeDatetime) && + (conEvalType == types.ETReal || conEvalType == types.ETDecimal || conEvalType == types.ETInt) +} + // Since the argument refining of cmp functions can bring some risks to the plan-cache, the optimizer // needs to decide to whether to skip the refining or skip plan-cache for safety. // For example, `unsigned_int_col > ?(-1)` can be refined to `True`, but the validation of this result @@ -1565,9 +1570,10 @@ func allowCmpArgsRefining4PlanCache(ctx sessionctx.Context, args []Expression) ( return true // plan-cache disabled or no parameter in these args } - // For these 2 cases below, we skip the refining: + // For these 3 cases below, we apply the refining: // 1. year-expr const // 2. int-expr string/float/double/decimal-const + // 3. datetime/timestamp column int/float/double/decimal-const for conIdx := 0; conIdx < 2; conIdx++ { if _, isCon := args[conIdx].(*Constant); !isCon { continue // not a constant @@ -1577,6 +1583,7 @@ func allowCmpArgsRefining4PlanCache(ctx sessionctx.Context, args []Expression) ( // refine `year < 12` to `year < 2012` to guarantee the correctness. // see https://github.com/pingcap/tidb/issues/41626 for more details. exprType := args[1-conIdx].GetType() + exprEvalType := exprType.EvalType() if exprType.GetType() == mysql.TypeYear { reason := errors.Errorf("'%v' may be converted to INT", args[conIdx].String()) ctx.GetSessionVars().StmtCtx.SetSkipPlanCache(reason) @@ -1585,27 +1592,39 @@ func allowCmpArgsRefining4PlanCache(ctx sessionctx.Context, args []Expression) ( // case 2: int-expr string/float/double/decimal-const // refine `int_key < 1.1` to `int_key < 2` to generate RangeScan instead of FullScan. - conType := args[conIdx].GetType().EvalType() - if exprType.EvalType() == types.ETInt && - (conType == types.ETString || conType == types.ETReal || conType == types.ETDecimal) { + conEvalType := args[conIdx].GetType().EvalType() + if exprEvalType == types.ETInt && + (conEvalType == types.ETString || conEvalType == types.ETReal || conEvalType == types.ETDecimal) { reason := errors.Errorf("'%v' may be converted to INT", args[conIdx].String()) ctx.GetSessionVars().StmtCtx.SetSkipPlanCache(reason) return true } + + // case 3: datetime/timestamp column int/float/double/decimal-const + // try refine numeric-const to timestamp const + // see https://github.com/pingcap/tidb/issues/38361 for more details + _, exprIsCon := args[1-conIdx].(*Constant) + if !exprIsCon && matchRefineRule3Pattern(conEvalType, exprType) { + reason := errors.Errorf("'%v' may be converted to datetime", args[conIdx].String()) + ctx.GetSessionVars().StmtCtx.SetSkipPlanCache(reason) + return true + } } return false } -// refineArg will rewrite the arguments if the compare expression is `int column non-int constant` or -// `non-int constant int column`. E.g., `a < 1.1` will be rewritten to `a < 2`. It also handles comparing year type -// with int constant if the int constant falls into a sensible year representation. -// This refine operation depends on the values of these args, but these values can change when using plan-cache. +// refineArgs will rewrite the arguments if the compare expression is +// 1. `int column non-int constant` or `non-int constant int column`. E.g., `a < 1.1` will be rewritten to `a < 2`. +// 2. It also handles comparing year type with int constant if the int constant falls into a sensible year representation. +// 3. It also handles comparing datetime/timestamp column with numeric constant, try to cast numeric constant as timestamp type, do nothing if failed. +// This refining operation depends on the values of these args, but these values can change when using plan-cache. // So we have to skip this operation or mark the plan as over-optimized when using plan-cache. func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Expression) []Expression { arg0Type, arg1Type := args[0].GetType(), args[1].GetType() - arg0IsInt := arg0Type.EvalType() == types.ETInt - arg1IsInt := arg1Type.EvalType() == types.ETInt + arg0EvalType, arg1EvalType := arg0Type.EvalType(), arg1Type.EvalType() + arg0IsInt := arg0EvalType == types.ETInt + arg1IsInt := arg1EvalType == types.ETInt arg0, arg0IsCon := args[0].(*Constant) arg1, arg1IsCon := args[1].(*Constant) isExceptional, finalArg0, finalArg1 := false, args[0], args[1] @@ -1617,6 +1636,14 @@ func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Express // We should remove the mutable constant for correctness, because its value may be changed. RemoveMutableConst(ctx, args) + if arg0IsCon && !arg1IsCon && matchRefineRule3Pattern(arg0EvalType, arg1Type) { + return c.refineNumericConstantCmpDatetime(ctx, args, arg0, 0) + } + + if !arg0IsCon && arg1IsCon && matchRefineRule3Pattern(arg1EvalType, arg0Type) { + return c.refineNumericConstantCmpDatetime(ctx, args, arg1, 1) + } + // int non-constant [cmp] non-int constant if arg0IsInt && !arg0IsCon && !arg1IsInt && arg1IsCon { arg1, isExceptional = RefineComparedConstant(ctx, *arg0Type, arg1, c.op) @@ -1661,6 +1688,7 @@ func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Express } } } + // int constant [cmp] year type if arg0IsCon && arg0IsInt && arg1Type.GetType() == mysql.TypeYear && !arg0.Value.IsNull() { adjusted, failed := types.AdjustYear(arg0.Value.GetInt64(), false) @@ -1699,6 +1727,31 @@ func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Express return c.refineArgsByUnsignedFlag(ctx, []Expression{finalArg0, finalArg1}) } +// see https://github.com/pingcap/tidb/issues/38361 for more details +func (c *compareFunctionClass) refineNumericConstantCmpDatetime(ctx sessionctx.Context, args []Expression, constArg *Constant, constArgIdx int) []Expression { + dt, err := constArg.Eval(chunk.Row{}) + if err != nil || dt.IsNull() { + return args + } + sc := ctx.GetSessionVars().StmtCtx + var datetimeDatum types.Datum + targetFieldType := types.NewFieldType(mysql.TypeDatetime) + datetimeDatum, err = dt.ConvertTo(sc, targetFieldType) + if err != nil || datetimeDatum.IsNull() { + return args + } + finalArg := Constant{ + Value: datetimeDatum, + RetType: targetFieldType, + DeferredExpr: nil, + ParamMarker: nil, + } + if constArgIdx == 0 { + return []Expression{&finalArg, args[1]} + } + return []Expression{args[0], &finalArg} +} + func (c *compareFunctionClass) refineArgsByUnsignedFlag(ctx sessionctx.Context, args []Expression) []Expression { // Only handle int cases, cause MySQL declares that `UNSIGNED` is deprecated for FLOAT, DOUBLE and DECIMAL types, // and support for it would be removed in a future version. From 63d6518fa736a1ced7dbb331ccb4f064181bf022 Mon Sep 17 00:00:00 2001 From: yibin Date: Wed, 16 Aug 2023 10:54:58 +0800 Subject: [PATCH 3/4] Revert executor_test changes Signed-off-by: yibin --- executor/executor_test.go | 42 --------------------------------------- 1 file changed, 42 deletions(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index f24de4bdbade5..de43d423c39a6 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -6511,45 +6511,3 @@ func TestProcessInfoOfSubQuery(t *testing.T) { tk2.MustQuery("select 1 from information_schema.processlist where TxnStart != '' and info like 'select%sleep% from t%'").Check(testkit.Rows("1")) wg.Wait() } - -func TestCompareIssue38361(t *testing.T) { - store := testkit.CreateMockStore(t) - - tk := testkit.NewTestKit(t, store) - tk.MustExec("drop database if exists TEST1") - tk.MustExec("create database TEST1") - tk.MustExec("use TEST1") - tk.MustExec("create table t(a datetime, b bigint, c bigint)") - tk.MustExec("insert into t values(cast('2023-08-09 00:00:00' as datetime), 20230809, 20231310)") - - tk.MustQuery("select a > 20230809 from t").Check(testkit.Rows("0")) - tk.MustQuery("select a = 20230809 from t").Check(testkit.Rows("1")) - tk.MustQuery("select a < 20230810 from t").Check(testkit.Rows("1")) - //// 20231310 can't be converted to valid datetime, thus should be compared using real date type,and datetime will be - //// converted to something like 'YYYYMMDDHHMMSS', bigger than 20231310 - tk.MustQuery("select a < 20231310 from t").Check(testkit.Rows("0")) - tk.MustQuery("select 20230809 < a from t").Check(testkit.Rows("0")) - tk.MustQuery("select 20230809 = a from t").Check(testkit.Rows("1")) - tk.MustQuery("select 20230810 > a from t").Check(testkit.Rows("1")) - tk.MustQuery("select 20231310 > a from t").Check(testkit.Rows("0")) - - //// constant datetime cmp numeric constant should be compared as real data type - tk.MustQuery("select cast('2023-08-09 00:00:00' as datetime) > 20230809 from t").Check(testkit.Rows("1")) - tk.MustQuery("select cast('2023-08-09 00:00:00' as datetime) = 20230809 from t").Check(testkit.Rows("0")) - tk.MustQuery("select cast('2023-08-09 00:00:00' as datetime) < 20230810 from t").Check(testkit.Rows("0")) - tk.MustQuery("select cast('2023-08-09 00:00:00' as datetime) < 20231310 from t").Check(testkit.Rows("0")) - tk.MustQuery("select 20230809 < cast('2023-08-09 00:00:00' as datetime) from t").Check(testkit.Rows("1")) - tk.MustQuery("select 20230809 = cast('2023-08-09 00:00:00' as datetime) from t").Check(testkit.Rows("0")) - tk.MustQuery("select 20230810 > cast('2023-08-09 00:00:00' as datetime) from t").Check(testkit.Rows("0")) - tk.MustQuery("select 20231310 > cast('2023-08-09 00:00:00' as datetime) from t").Check(testkit.Rows("0")) - - //// datetime column cmp numeric column should be compared as real data type - tk.MustQuery("select a > b from t").Check(testkit.Rows("1")) - tk.MustQuery("select a = b from t").Check(testkit.Rows("0")) - tk.MustQuery("select a < b + 1 from t").Check(testkit.Rows("0")) - tk.MustQuery("select a < c from t").Check(testkit.Rows("0")) - tk.MustQuery("select b < a from t").Check(testkit.Rows("1")) - tk.MustQuery("select b = a from t").Check(testkit.Rows("0")) - tk.MustQuery("select b > a from t").Check(testkit.Rows("0")) - tk.MustQuery("select c > a from t").Check(testkit.Rows("0")) -} From ca7fd94fa795087d9af39c6fac6a9b9be5954552 Mon Sep 17 00:00:00 2001 From: yibin Date: Wed, 16 Aug 2023 11:36:58 +0800 Subject: [PATCH 4/4] Move unit test to planner Signed-off-by: yibin --- planner/core/expression_rewriter_test.go | 42 ++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/planner/core/expression_rewriter_test.go b/planner/core/expression_rewriter_test.go index 93a9d72bfcc3a..79d4e0798e6ce 100644 --- a/planner/core/expression_rewriter_test.go +++ b/planner/core/expression_rewriter_test.go @@ -392,3 +392,45 @@ func TestConvertIfNullToCast(t *testing.T) { "[ └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo", )) } + +func TestCompareIssue38361(t *testing.T) { + store := testkit.CreateMockStore(t) + + tk := testkit.NewTestKit(t, store) + tk.MustExec("drop database if exists TEST1") + tk.MustExec("create database TEST1") + tk.MustExec("use TEST1") + tk.MustExec("create table t(a datetime, b bigint, c bigint)") + tk.MustExec("insert into t values(cast('2023-08-09 00:00:00' as datetime), 20230809, 20231310)") + + tk.MustQuery("select a > 20230809 from t").Check(testkit.Rows("0")) + tk.MustQuery("select a = 20230809 from t").Check(testkit.Rows("1")) + tk.MustQuery("select a < 20230810 from t").Check(testkit.Rows("1")) + //// 20231310 can't be converted to valid datetime, thus should be compared using real date type,and datetime will be + //// converted to something like 'YYYYMMDDHHMMSS', bigger than 20231310 + tk.MustQuery("select a < 20231310 from t").Check(testkit.Rows("0")) + tk.MustQuery("select 20230809 < a from t").Check(testkit.Rows("0")) + tk.MustQuery("select 20230809 = a from t").Check(testkit.Rows("1")) + tk.MustQuery("select 20230810 > a from t").Check(testkit.Rows("1")) + tk.MustQuery("select 20231310 > a from t").Check(testkit.Rows("0")) + + //// constant datetime cmp numeric constant should be compared as real data type + tk.MustQuery("select cast('2023-08-09 00:00:00' as datetime) > 20230809 from t").Check(testkit.Rows("1")) + tk.MustQuery("select cast('2023-08-09 00:00:00' as datetime) = 20230809 from t").Check(testkit.Rows("0")) + tk.MustQuery("select cast('2023-08-09 00:00:00' as datetime) < 20230810 from t").Check(testkit.Rows("0")) + tk.MustQuery("select cast('2023-08-09 00:00:00' as datetime) < 20231310 from t").Check(testkit.Rows("0")) + tk.MustQuery("select 20230809 < cast('2023-08-09 00:00:00' as datetime) from t").Check(testkit.Rows("1")) + tk.MustQuery("select 20230809 = cast('2023-08-09 00:00:00' as datetime) from t").Check(testkit.Rows("0")) + tk.MustQuery("select 20230810 > cast('2023-08-09 00:00:00' as datetime) from t").Check(testkit.Rows("0")) + tk.MustQuery("select 20231310 > cast('2023-08-09 00:00:00' as datetime) from t").Check(testkit.Rows("0")) + + //// datetime column cmp numeric column should be compared as real data type + tk.MustQuery("select a > b from t").Check(testkit.Rows("1")) + tk.MustQuery("select a = b from t").Check(testkit.Rows("0")) + tk.MustQuery("select a < b + 1 from t").Check(testkit.Rows("0")) + tk.MustQuery("select a < c from t").Check(testkit.Rows("0")) + tk.MustQuery("select b < a from t").Check(testkit.Rows("1")) + tk.MustQuery("select b = a from t").Check(testkit.Rows("0")) + tk.MustQuery("select b > a from t").Check(testkit.Rows("0")) + tk.MustQuery("select c > a from t").Check(testkit.Rows("0")) +}