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: let PushDownNot does not change the argument #10363

Merged
merged 3 commits into from
May 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions cmd/explaintest/r/explain_easy.result
Original file line number Diff line number Diff line change
Expand Up @@ -643,3 +643,20 @@ id count task operator info
Point_Get_1 1.00 root table:t, index:i j
rollback;
drop table if exists t;
create table t(a int);
begin;
insert into t values (1);
explain select * from t left outer join t t1 on t.a = t1.a where t.a not between 1 and 2;
id count task operator info
Projection_8 8320.83 root test.t.a, test.t1.a
└─HashLeftJoin_9 8320.83 root left outer join, inner:UnionScan_14, equal:[eq(test.t.a, test.t1.a)]
├─UnionScan_10 6656.67 root not(and(ge(test.t.a, 1), le(test.t.a, 2)))
│ └─TableReader_13 6656.67 root data:Selection_12
│ └─Selection_12 6656.67 cop or(lt(test.t.a, 1), gt(test.t.a, 2))
│ └─TableScan_11 10000.00 cop table:t, range:[-inf,+inf], keep order:false, stats:pseudo
└─UnionScan_14 6656.67 root not(and(ge(test.t1.a, 1), le(test.t1.a, 2))), not(isnull(test.t1.a))
└─TableReader_17 6656.67 root data:Selection_16
└─Selection_16 6656.67 cop not(isnull(test.t1.a)), or(lt(test.t1.a, 1), gt(test.t1.a, 2))
└─TableScan_15 10000.00 cop table:t, range:[-inf,+inf], keep order:false, stats:pseudo
rollback;
drop table if exists t;
8 changes: 8 additions & 0 deletions cmd/explaintest/t/explain_easy.test
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,11 @@ insert into t values (1, 1);
explain update t set j = -j where i = 1 and j = 1;
rollback;
drop table if exists t;

# https://github.com/pingcap/tidb/issues/10344
create table t(a int);
begin;
insert into t values (1);
explain select * from t left outer join t t1 on t.a = t1.a where t.a not between 1 and 2;
rollback;
drop table if exists t;
40 changes: 18 additions & 22 deletions expression/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,14 @@ var symmetricOp = map[opcode.Op]opcode.Op{
opcode.NullEQ: opcode.NullEQ,
}

func doPushDownNot(ctx sessionctx.Context, exprs []Expression, not bool) []Expression {
newExprs := make([]Expression, 0, len(exprs))
for _, expr := range exprs {
newExprs = append(newExprs, PushDownNot(ctx, expr, not))
}
return newExprs
}

// PushDownNot pushes the `not` function down to the expression's arguments.
func PushDownNot(ctx sessionctx.Context, expr Expression, not bool) Expression {
if f, ok := expr.(*ScalarFunction); ok {
Expand All @@ -299,34 +307,22 @@ func PushDownNot(ctx sessionctx.Context, expr Expression, not bool) Expression {
if not {
return NewFunctionInternal(f.GetCtx(), oppositeOp[f.FuncName.L], f.GetType(), f.GetArgs()...)
}
for i, arg := range f.GetArgs() {
f.GetArgs()[i] = PushDownNot(f.GetCtx(), arg, false)
}
return f
newArgs := doPushDownNot(f.GetCtx(), f.GetArgs(), false)
return NewFunctionInternal(f.GetCtx(), f.FuncName.L, f.GetType(), newArgs...)
case ast.LogicAnd:
if not {
args := f.GetArgs()
for i, a := range args {
args[i] = PushDownNot(f.GetCtx(), a, true)
}
return NewFunctionInternal(f.GetCtx(), ast.LogicOr, f.GetType(), args...)
}
for i, arg := range f.GetArgs() {
f.GetArgs()[i] = PushDownNot(f.GetCtx(), arg, false)
newArgs := doPushDownNot(f.GetCtx(), f.GetArgs(), true)
return NewFunctionInternal(f.GetCtx(), ast.LogicOr, f.GetType(), newArgs...)
}
return f
newArgs := doPushDownNot(f.GetCtx(), f.GetArgs(), false)
return NewFunctionInternal(f.GetCtx(), f.FuncName.L, f.GetType(), newArgs...)
case ast.LogicOr:
if not {
args := f.GetArgs()
for i, a := range args {
args[i] = PushDownNot(f.GetCtx(), a, true)
}
return NewFunctionInternal(f.GetCtx(), ast.LogicAnd, f.GetType(), args...)
}
for i, arg := range f.GetArgs() {
f.GetArgs()[i] = PushDownNot(f.GetCtx(), arg, false)
newArgs := doPushDownNot(f.GetCtx(), f.GetArgs(), true)
return NewFunctionInternal(f.GetCtx(), ast.LogicAnd, f.GetType(), newArgs...)
}
return f
newArgs := doPushDownNot(f.GetCtx(), f.GetArgs(), false)
return NewFunctionInternal(f.GetCtx(), f.FuncName.L, f.GetType(), newArgs...)
}
}
if not {
Expand Down
2 changes: 2 additions & 0 deletions expression/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,10 @@ func (s *testUtilSuite) TestPushDownNot(c *check.C) {
neFunc := newFunction(ast.NE, col, One)
andFunc2 := newFunction(ast.LogicAnd, neFunc, neFunc)
orFunc2 := newFunction(ast.LogicOr, andFunc2, neFunc)
notFuncCopy := notFunc.Clone()
ret := PushDownNot(ctx, notFunc, false)
c.Assert(ret.Equal(ctx, orFunc2), check.IsTrue)
c.Assert(notFunc.Equal(ctx, notFuncCopy), check.IsTrue)
}

func (s *testUtilSuite) TestFilter(c *check.C) {
Expand Down
2 changes: 1 addition & 1 deletion planner/core/logical_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ func (s *testPlanSuite) TestSimplifyOuterJoin(c *C) {
},
{
sql: "select * from t t1 left join t t2 on t1.b = t2.b where not (t1.c > 1 and t2.c > 1);",
best: "Join{DataScan(t1)->DataScan(t2)}(test.t1.b,test.t2.b)->Sel([not(and(le(test.t1.c, 1), le(test.t2.c, 1)))])->Projection",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this one is wrong previously? 😂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes...

best: "Join{DataScan(t1)->DataScan(t2)}(test.t1.b,test.t2.b)->Sel([not(and(gt(test.t1.c, 1), gt(test.t2.c, 1)))])->Projection",
joinType: "left outer join",
},
{
Expand Down