Skip to content

Commit

Permalink
planner: revise isnullRejected check for And and OR (#38430)
Browse files Browse the repository at this point in the history
close #38304
  • Loading branch information
Yisaer authored Oct 13, 2022
1 parent af714c2 commit 84fbfca
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 10 deletions.
68 changes: 68 additions & 0 deletions expression/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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)
Expand Down
16 changes: 16 additions & 0 deletions planner/core/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 <nil>"))
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 <nil>"))
tk.MustQuery("SELECT * FROM t0 LEFT JOIN t1 ON NULL WHERE t1.c0 or true; ").Check(testkit.Rows("> 1 <nil>"))
tk.MustQuery("SELECT * FROM t0 LEFT JOIN t1 ON NULL WHERE not(t1.c0 and false); ").Check(testkit.Rows("> 1 <nil>"))
}
24 changes: 14 additions & 10 deletions planner/core/rule_predicate_push_down.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit 84fbfca

Please sign in to comment.