From 69b2e4bd3ce4e082e53d8a0c3f8f4bc8c60dd06b Mon Sep 17 00:00:00 2001 From: yisaer Date: Fri, 28 Oct 2022 10:56:24 +0800 Subject: [PATCH 1/2] fix and test Signed-off-by: yisaer --- expression/expression.go | 52 +++++++++++++++++++++------------------ planner/core/plan_test.go | 7 ++++++ 2 files changed, 35 insertions(+), 24 deletions(-) diff --git a/expression/expression.go b/expression/expression.go index e4b8ae764a2bd..673026422c0e1 100644 --- a/expression/expression.go +++ b/expression/expression.go @@ -858,49 +858,53 @@ func evaluateExprWithNullInNullRejectCheck(ctx sessionctx.Context, schema *Schem for i, arg := range x.GetArgs() { args[i], nullFromSets[i] = evaluateExprWithNullInNullRejectCheck(ctx, schema, arg) } - - // allNullFromSet indicates whether all arguments are Null Constant and the Null Constant is affected by the column of the schema. - allNullFromSet := true + allArgsNullFromSet := true for i := range args { if cons, ok := args[i].(*Constant); ok && cons.Value.IsNull() && !nullFromSets[i] { - allNullFromSet = false + allArgsNullFromSet = false break } } - - // allArgsNullFromSet indicates whether all Null Constant are affected by the column of the schema - allArgsNullFromSet := true - for i := range args { - if cons, ok := args[i].(*Constant); ok && cons.Value.IsNull() && nullFromSets[i] { - continue + if x.FuncName.L == ast.LogicAnd || x.FuncName.L == ast.LogicOr { + hasNonConstantArg := false + for _, arg := range args { + if _, ok := arg.(*Constant); !ok { + hasNonConstantArg = true + break + } + } + if hasNonConstantArg { + for i := range args { + if cons, ok := args[i].(*Constant); ok && cons.Value.IsNull() && nullFromSets[i] { + if x.FuncName.L == ast.LogicAnd { + args[i] = NewOne() + break + } + if x.FuncName.L == ast.LogicOr { + args[i] = NewZero() + break + } + } + } } - allArgsNullFromSet = false } // If all the args are Null Constant and affected by the column schema, then we should keep it. // Otherwise, we shouldn't let Null Constant which affected by the column schema participate in computing in `And` and `OR` // due to the result of `AND` and `OR are uncertain if one of the arguments is NULL. - if !allArgsNullFromSet { - for i := range args { - if cons, ok := args[i].(*Constant); ok && cons.Value.IsNull() && nullFromSets[i] { - if x.FuncName.L == ast.LogicAnd { - args[i] = NewOne() - } - if x.FuncName.L == ast.LogicOr { - args[i] = NewZero() - } - } - } - } + c := NewFunctionInternal(ctx, x.FuncName.L, x.RetType.Clone(), args...) cons, ok := c.(*Constant) // If the return expr is Null Constant, and all the Null Constant arguments are affected by column schema, // then we think the result Null Constant is also affected by the column schema - return c, ok && cons.Value.IsNull() && allNullFromSet + return c, ok && cons.Value.IsNull() && allArgsNullFromSet case *Column: if !schema.Contains(x) { return x, false } + if ctx.GetSessionVars().StmtCtx.OptimizeTracer != nil { + fmt.Println() + } return &Constant{Value: types.Datum{}, RetType: types.NewFieldType(mysql.TypeNull)}, true case *Constant: if x.DeferredExpr != nil { diff --git a/planner/core/plan_test.go b/planner/core/plan_test.go index ebbb9c7373401..54b6d0ad3e381 100644 --- a/planner/core/plan_test.go +++ b/planner/core/plan_test.go @@ -1078,6 +1078,7 @@ func TestNullEQConditionPlan(t *testing.T) { } // https://github.com/pingcap/tidb/issues/38304 +// https://github.com/pingcap/tidb/issues/38654 func TestOuterJoinOnNull(t *testing.T) { store := testkit.CreateMockStore(t) tk := testkit.NewTestKit(t, store) @@ -1091,4 +1092,10 @@ func TestOuterJoinOnNull(t *testing.T) { tk.MustQuery("SELECT * FROM t0 LEFT JOIN t1 ON NULL WHERE NOT '2' =(t1.c0 AND t0.c1 IS NULL); ").Check(testkit.Rows("> 1 ")) tk.MustQuery("SELECT * FROM t0 LEFT JOIN t1 ON NULL WHERE t1.c0 or true; ").Check(testkit.Rows("> 1 ")) tk.MustQuery("SELECT * FROM t0 LEFT JOIN t1 ON NULL WHERE not(t1.c0 and false); ").Check(testkit.Rows("> 1 ")) + + tk.MustExec("CREATE TABLE t2(c0 INT);") + tk.MustExec("CREATE TABLE t3(c0 INT);") + tk.MustExec("INSERT INTO t3 VALUES (1);") + tk.MustQuery("SELECT ((NOT ('i'))AND(t2.c0)) IS NULL FROM t2 RIGHT JOIN t3 ON t3.c0;").Check(testkit.Rows("1")) + tk.MustQuery("SELECT * FROM t2 RIGHT JOIN t3 ON t2.c0 WHERE ((NOT ('i'))AND(t2.c0)) IS NULL;").Check(testkit.Rows(" 1")) } From 6a1141958dc72bb3392572a1ff341b0838871551 Mon Sep 17 00:00:00 2001 From: yisaer Date: Fri, 28 Oct 2022 11:00:23 +0800 Subject: [PATCH 2/2] add comment Signed-off-by: yisaer --- expression/expression.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/expression/expression.go b/expression/expression.go index 673026422c0e1..fefa1c403c959 100644 --- a/expression/expression.go +++ b/expression/expression.go @@ -865,6 +865,10 @@ func evaluateExprWithNullInNullRejectCheck(ctx sessionctx.Context, schema *Schem break } } + + // If one of the args of `AND` and `OR` are Null Constant from the column schema, and the another argument is Constant, then we should keep it. + // Otherwise, we shouldn't let Null Constant which affected by the column schema participate in computing in `And` and `OR` + // due to the result of `AND` and `OR` are uncertain if one of the arguments is NULL. if x.FuncName.L == ast.LogicAnd || x.FuncName.L == ast.LogicOr { hasNonConstantArg := false for _, arg := range args { @@ -889,10 +893,6 @@ func evaluateExprWithNullInNullRejectCheck(ctx sessionctx.Context, schema *Schem } } - // If all the args are Null Constant and affected by the column schema, then we should keep it. - // Otherwise, we shouldn't let Null Constant which affected by the column schema participate in computing in `And` and `OR` - // due to the result of `AND` and `OR are uncertain if one of the arguments is NULL. - c := NewFunctionInternal(ctx, x.FuncName.L, x.RetType.Clone(), args...) cons, ok := c.(*Constant) // If the return expr is Null Constant, and all the Null Constant arguments are affected by column schema, @@ -902,9 +902,6 @@ func evaluateExprWithNullInNullRejectCheck(ctx sessionctx.Context, schema *Schem if !schema.Contains(x) { return x, false } - if ctx.GetSessionVars().StmtCtx.OptimizeTracer != nil { - fmt.Println() - } return &Constant{Value: types.Datum{}, RetType: types.NewFieldType(mysql.TypeNull)}, true case *Constant: if x.DeferredExpr != nil {