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

expression: fix error PushDownNot for contiguous NOT #16108

Merged
merged 15 commits into from
Apr 15, 2020
10 changes: 10 additions & 0 deletions expression/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5971,3 +5971,13 @@ func (s *testIntegrationSuite) TestNegativeZeroForHashJoin(c *C) {
tk.MustExec("drop TABLE t0;")
tk.MustExec("drop table t1;")
}

func (s *testIntegrationSuite) TestIssue15725(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test;")
tk.MustExec("drop table if exists t")
tk.MustExec("create table t(a int)")
tk.MustExec("insert into t values(2)")
tk.MustQuery("select * from t where (not not a) = a").Check(testkit.Rows())
tk.MustQuery("select * from t where (not not not not a) = a").Check(testkit.Rows())
}
42 changes: 39 additions & 3 deletions expression/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,24 @@ func timeZone2Duration(tz string) time.Duration {
return time.Duration(sign) * (time.Duration(h)*time.Hour + time.Duration(m)*time.Minute)
}

var logicalOps = map[string]struct{}{
ast.LT: {},
ast.GE: {},
ast.GT: {},
ast.LE: {},
ast.EQ: {},
ast.NE: {},
ast.UnaryNot: {},
ast.LogicAnd: {},
ast.LogicOr: {},
ast.LogicXor: {},
ast.In: {},
ast.IsNull: {},
ast.IsTruth: {},
ast.IsFalsity: {},
ast.Like: {},
}

var oppositeOp = map[string]string{
ast.LT: ast.GE,
ast.GE: ast.LT,
Expand Down Expand Up @@ -369,12 +387,30 @@ func pushNotAcrossArgs(ctx sessionctx.Context, exprs []Expression, not bool) ([]
return newExprs, flag
}

// pushNotAcrossExpr try to eliminate the NOT expr in expression tree. It will records whether there's already NOT pushed.
func pushNotAcrossExpr(ctx sessionctx.Context, expr Expression, not bool) (Expression, bool) {
// pushNotAcrossExpr try to eliminate the NOT expr in expression tree.
// Input `not` indicates whether there's a `NOT` be pushed down.
// Output `changed` indicates whether the output expression differs from the
// input `expr` because of the pushed-down-not.
func pushNotAcrossExpr(ctx sessionctx.Context, expr Expression, not bool) (_ Expression, changed bool) {
if f, ok := expr.(*ScalarFunction); ok {
switch f.FuncName.L {
case ast.UnaryNot:
return pushNotAcrossExpr(f.GetCtx(), f.GetArgs()[0], !not)
var childExpr Expression
eurekaka marked this conversation as resolved.
Show resolved Hide resolved
// UnaryNot only returns 0/1/NULL, thus we should not eliminate the
// innermost NOT if the arg is not a logical operator.
switch child := f.GetArgs()[0].(type) {
case *ScalarFunction:
if _, isLogicalOp := logicalOps[child.FuncName.L]; !isLogicalOp {
eurekaka marked this conversation as resolved.
Show resolved Hide resolved
return expr, false
}
childExpr, changed = pushNotAcrossExpr(f.GetCtx(), f.GetArgs()[0], !not)
if !changed && !not {
return expr, false
}
return childExpr, true
case *Column:
return expr, false
}
case ast.LT, ast.GE, ast.GT, ast.LE, ast.EQ, ast.NE:
if not {
return NewFunctionInternal(f.GetCtx(), oppositeOp[f.FuncName.L], f.GetType(), f.GetArgs()...), true
Expand Down
29 changes: 29 additions & 0 deletions expression/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,35 @@ func (s *testUtilSuite) TestPushDownNot(c *check.C) {
ret := PushDownNot(ctx, notFunc)
c.Assert(ret.Equal(ctx, orFunc2), check.IsTrue)
c.Assert(notFunc.Equal(ctx, notFuncCopy), check.IsTrue)

// issue 15725
// (not not a) should not be optimized to (a)
notFunc = newFunction(ast.UnaryNot, col)
notFunc = newFunction(ast.UnaryNot, notFunc)
ret = PushDownNot(ctx, notFunc)
c.Assert(ret.Equal(ctx, col), check.IsFalse)

// (not not (a+1)) should not be optimized to (a+1)
plusFunc := newFunction(ast.Plus, col, One)
notFunc = newFunction(ast.UnaryNot, plusFunc)
notFunc = newFunction(ast.UnaryNot, notFunc)
ret = PushDownNot(ctx, notFunc)
c.Assert(ret.Equal(ctx, col), check.IsFalse)

// (not not not a) should be optimized to (not a)
notFunc = newFunction(ast.UnaryNot, col)
notFunc = newFunction(ast.UnaryNot, notFunc)
notFunc = newFunction(ast.UnaryNot, notFunc)
ret = PushDownNot(ctx, notFunc)
c.Assert(ret.Equal(ctx, newFunction(ast.UnaryNot, col)), check.IsTrue)

// (not not not not a) should be optimized to (not not a)
notFunc = newFunction(ast.UnaryNot, col)
notFunc = newFunction(ast.UnaryNot, notFunc)
notFunc = newFunction(ast.UnaryNot, notFunc)
notFunc = newFunction(ast.UnaryNot, notFunc)
ret = PushDownNot(ctx, notFunc)
c.Assert(ret.Equal(ctx, newFunction(ast.UnaryNot, newFunction(ast.UnaryNot, col))), check.IsTrue)
}

func (s *testUtilSuite) TestFilter(c *check.C) {
Expand Down