From 84fbfcada3d9564fe922771f010db318d27fc77a Mon Sep 17 00:00:00 2001 From: Song Gao Date: Fri, 14 Oct 2022 01:15:52 +0800 Subject: [PATCH] planner: revise isnullRejected check for `And` and `OR` (#38430) close pingcap/tidb#38304 --- expression/expression.go | 68 ++++++++++++++++++++++++ planner/core/plan_test.go | 16 ++++++ planner/core/rule_predicate_push_down.go | 24 +++++---- 3 files changed, 98 insertions(+), 10 deletions(-) diff --git a/expression/expression.go b/expression/expression.go index e8f38910c1264..e4b8ae764a2bd 100644 --- a/expression/expression.go +++ b/expression/expression.go @@ -818,6 +818,10 @@ func EvaluateExprWithNull(ctx sessionctx.Context, schema *Schema, expr Expressio if MaybeOverOptimized4PlanCache(ctx, []Expression{expr}) { return expr } + if ctx.GetSessionVars().StmtCtx.InNullRejectCheck { + expr, _ = evaluateExprWithNullInNullRejectCheck(ctx, schema, expr) + return expr + } return evaluateExprWithNull(ctx, schema, expr) } @@ -842,6 +846,70 @@ func evaluateExprWithNull(ctx sessionctx.Context, schema *Schema, expr Expressio return expr } +// evaluateExprWithNullInNullRejectCheck sets columns in schema as null and calculate the final result of the scalar function. +// If the Expression is a non-constant value, it means the result is unknown. +// The returned bool values indicates whether the value is influenced by the Null Constant transformed from schema column +// when the value is Null Constant. +func evaluateExprWithNullInNullRejectCheck(ctx sessionctx.Context, schema *Schema, expr Expression) (Expression, bool) { + switch x := expr.(type) { + case *ScalarFunction: + args := make([]Expression, len(x.GetArgs())) + nullFromSets := make([]bool, len(x.GetArgs())) + 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 + for i := range args { + if cons, ok := args[i].(*Constant); ok && cons.Value.IsNull() && !nullFromSets[i] { + allNullFromSet = 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 + } + 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 + case *Column: + if !schema.Contains(x) { + return x, false + } + return &Constant{Value: types.Datum{}, RetType: types.NewFieldType(mysql.TypeNull)}, true + case *Constant: + if x.DeferredExpr != nil { + return FoldConstant(x), false + } + } + return expr, false +} + // TableInfo2SchemaAndNames converts the TableInfo to the schema and name slice. func TableInfo2SchemaAndNames(ctx sessionctx.Context, dbName model.CIStr, tbl *model.TableInfo) (*Schema, []*types.FieldName, error) { cols, names, err := ColumnInfos2ColumnsAndNames(ctx, dbName, tbl.Name, tbl.Cols(), tbl) diff --git a/planner/core/plan_test.go b/planner/core/plan_test.go index 242d868a8c571..ebbb9c7373401 100644 --- a/planner/core/plan_test.go +++ b/planner/core/plan_test.go @@ -1076,3 +1076,19 @@ func TestNullEQConditionPlan(t *testing.T) { {"Point_Get_5", "root", "handle:0"}, }) } + +// https://github.com/pingcap/tidb/issues/38304 +func TestOuterJoinOnNull(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("CREATE TABLE t0(c0 BLOB(5), c1 BLOB(5));") + tk.MustExec("CREATE TABLE t1 (c0 BOOL);") + tk.MustExec("INSERT INTO t1 VALUES(false);") + tk.MustExec("INSERT INTO t0(c0, c1) VALUES ('>', true);") + tk.MustQuery("SELECT * FROM t0 LEFT OUTER JOIN t1 ON NULL; ").Check(testkit.Rows("> 1 ")) + tk.MustQuery("SELECT NOT '2' =(t1.c0 AND t0.c1 IS NULL) FROM t0 LEFT OUTER JOIN t1 ON NULL; ").Check(testkit.Rows("1")) + 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 ")) +} diff --git a/planner/core/rule_predicate_push_down.go b/planner/core/rule_predicate_push_down.go index 96d62942f2346..bebed1cab141a 100644 --- a/planner/core/rule_predicate_push_down.go +++ b/planner/core/rule_predicate_push_down.go @@ -440,16 +440,20 @@ func isNullRejected(ctx sessionctx.Context, schema *expression.Schema, expr expr } sc := ctx.GetSessionVars().StmtCtx sc.InNullRejectCheck = true - result := expression.EvaluateExprWithNull(ctx, schema, expr) - sc.InNullRejectCheck = false - x, ok := result.(*expression.Constant) - if !ok { - return false - } - if x.Value.IsNull() { - return true - } else if isTrue, err := x.Value.ToBool(sc); err == nil && isTrue == 0 { - return true + defer func() { + sc.InNullRejectCheck = false + }() + for _, cond := range expression.SplitCNFItems(expr) { + result := expression.EvaluateExprWithNull(ctx, schema, cond) + x, ok := result.(*expression.Constant) + if !ok { + continue + } + if x.Value.IsNull() { + return true + } else if isTrue, err := x.Value.ToBool(sc); err == nil && isTrue == 0 { + return true + } } return false }