Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

planner: revise isnullRejected check for And and OR #38430

Merged
merged 16 commits into from
Oct 13, 2022
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