From 02332b20de715c40b4363034f6f59c279ed2d257 Mon Sep 17 00:00:00 2001 From: YangKeao Date: Tue, 10 Jan 2023 04:04:23 -0500 Subject: [PATCH] expression, planner: exclude concat_ws from aggresive folding constant (#38383) close pingcap/tidb#36888 --- expression/constant_fold.go | 7 ++++++- expression/constant_test.go | 9 +++++++++ planner/core/integration_test.go | 13 +++++++++++++ 3 files changed, 28 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 7d37dfcfe13f7..81da36394968d 100644 --- a/planner/core/integration_test.go +++ b/planner/core/integration_test.go @@ -8309,6 +8309,19 @@ func TestAutoIncrementCheckWithCheckConstraint(t *testing.T) { )`) } +// 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("")) +} + +// https://github.com/pingcap/tidb/issues/40285. func TestIssue40285(t *testing.T) { store := testkit.CreateMockStore(t) tk := testkit.NewTestKit(t, store)