From ca4899d486a55c6ebd06a22795e61701ddf38049 Mon Sep 17 00:00:00 2001 From: YangKeao Date: Mon, 10 Oct 2022 23:12:40 +0800 Subject: [PATCH] exclude concat_ws from aggresive folding constant Signed-off-by: YangKeao --- expression/constant_fold.go | 7 ++++++- expression/constant_test.go | 9 +++++++++ planner/core/integration_test.go | 12 ++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/expression/constant_fold.go b/expression/constant_fold.go index f43cb75a1ff3c..0d73757b3161b 100644 --- a/expression/constant_fold.go +++ b/expression/constant_fold.go @@ -178,7 +178,12 @@ func foldConstant(expr Expression) (Expression, bool) { } } if !allConstArg { - if !hasNullArg || !sc.InNullRejectCheck || x.FuncName.L == ast.NullEQ { + // try to optimize on the situation when not all arguments are const + // for most functions, if one of the arguments are NULL, the result can be a constant (NULL or something else) + // + // NullEQ and ConcatWS are excluded, because they could have different value when the non-constant value is + // 1 or NULL. For example, concat_ws(NULL, NULL) gives NULL, but concat_ws(1, NULL) gives '' + if !hasNullArg || !sc.InNullRejectCheck || x.FuncName.L == ast.NullEQ || x.FuncName.L == ast.ConcatWS { return expr, isDeferredConst } constArgs := make([]Expression, len(args)) diff --git a/expression/constant_test.go b/expression/constant_test.go index 82c3d8e489fdf..2c2e226eabb5c 100644 --- a/expression/constant_test.go +++ b/expression/constant_test.go @@ -217,6 +217,15 @@ func TestConstantFolding(t *testing.T) { condition: newFunction(ast.LT, newColumn(0), newFunction(ast.Plus, newColumn(1), newFunction(ast.Plus, newLonglong(2), newLonglong(1)))), result: "lt(Column#0, plus(Column#1, 3))", }, + { + condition: func() Expression { + expr := newFunction(ast.ConcatWS, newColumn(0), NewNull()) + function := expr.(*ScalarFunction) + function.GetCtx().GetSessionVars().StmtCtx.InNullRejectCheck = true + return function + }(), + result: "concat_ws(cast(Column#0, var_string(20)), )", + }, } for _, tt := range tests { newConds := FoldConstant(tt.condition) diff --git a/planner/core/integration_test.go b/planner/core/integration_test.go index a89b85b736d62..5f9d14275262e 100644 --- a/planner/core/integration_test.go +++ b/planner/core/integration_test.go @@ -7823,3 +7823,15 @@ func TestNullConditionForPrefixIndex(t *testing.T) { " └─StreamAgg_9 1.00 cop[tikv] funcs:count(1)->Column#7", " └─IndexRangeScan_17 99.90 cop[tikv] table:t1, index:idx2(c1, c2) range:[\"0xfff\" -inf,\"0xfff\" +inf], keep order:false, stats:pseudo")) } + +// https://github.com/pingcap/tidb/issues/36888. +func TestIssue36888(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("CREATE TABLE t0(c0 INT);") + tk.MustExec("CREATE TABLE t1(c0 INT);") + + tk.MustExec("INSERT INTO t0 VALUES (NULL);") + tk.MustQuery("SELECT t0.c0 FROM t0 LEFT JOIN t1 ON t0.c0>=t1.c0 WHERE (CONCAT_WS(t0.c0, t1.c0) IS NULL);").Check(testkit.Rows("")) +}