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
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -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;
@@ -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;
@@ -289,6 +289,14 @@ var symmetricOp = map[opcode.Op]opcode.Op{
opcode.NullEQ: opcode.NullEQ,
}

func pushDownNot(ctx sessionctx.Context, exprs []Expression, not bool) []Expression {
This conversation was marked as resolved by lamxTyler

This comment has been minimized.

Copy link
@XuHuaiyu

XuHuaiyu May 6, 2019

Contributor

doPushDownNot ?

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 {
@@ -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 := pushDownNot(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 := pushDownNot(f.GetCtx(), f.GetArgs(), true)
return NewFunctionInternal(f.GetCtx(), ast.LogicOr, f.GetType(), newArgs...)
}
return f
newArgs := pushDownNot(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 := pushDownNot(f.GetCtx(), f.GetArgs(), true)
return NewFunctionInternal(f.GetCtx(), ast.LogicAnd, f.GetType(), newArgs...)
}
return f
newArgs := pushDownNot(f.GetCtx(), f.GetArgs(), false)
return NewFunctionInternal(f.GetCtx(), f.FuncName.L, f.GetType(), newArgs...)
}
}
if not {
@@ -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) {
@@ -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",

This comment has been minimized.

Copy link
@winoros

winoros May 6, 2019

Member

So this one is wrong previously? 😂

This comment has been minimized.

Copy link
@lamxTyler

lamxTyler May 6, 2019

Author Member

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",
},
{
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.