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.

Sign up for GitHub

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

Jump to bottom

expression: let PushDownNot does not change the argument #10363

Merged
merged 3 commits into from
May 6, 2019
Merged

expression: let PushDownNot does not change the argument #10363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b7a5cdd
expression: let `PushDownNot` does not change the argument
alivxxx May 6, 2019
dd19ae6
address comments
alivxxx May 6, 2019
5563401
Merge branch 'master' into pushNot
alivxxx May 6, 2019
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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