From 0106361cd92da0a5f79557d33613841bab9271b3 Mon Sep 17 00:00:00 2001 From: Haibin Xie Date: Mon, 6 May 2019 16:22:19 +0800 Subject: [PATCH] expression: let `PushDownNot` does not change the argument (#10363) --- cmd/explaintest/r/explain_easy.result | 17 ++++++++++++ cmd/explaintest/t/explain_easy.test | 8 ++++++ expression/util.go | 40 ++++++++++++--------------- expression/util_test.go | 2 ++ planner/core/logical_plan_test.go | 2 +- 5 files changed, 46 insertions(+), 23 deletions(-) diff --git a/cmd/explaintest/r/explain_easy.result b/cmd/explaintest/r/explain_easy.result index 5255df34cea89..4806f6be3d3b7 100644 --- a/cmd/explaintest/r/explain_easy.result +++ b/cmd/explaintest/r/explain_easy.result @@ -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; diff --git a/cmd/explaintest/t/explain_easy.test b/cmd/explaintest/t/explain_easy.test index 8c2b399be3115..a8cd985164413 100644 --- a/cmd/explaintest/t/explain_easy.test +++ b/cmd/explaintest/t/explain_easy.test @@ -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; diff --git a/expression/util.go b/expression/util.go index 881a8baa19a9b..368a211d2c472 100644 --- a/expression/util.go +++ b/expression/util.go @@ -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 { @@ -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 { diff --git a/expression/util_test.go b/expression/util_test.go index 1d9f9b936860f..bf6ff7f459cb0 100644 --- a/expression/util_test.go +++ b/expression/util_test.go @@ -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) { diff --git a/planner/core/logical_plan_test.go b/planner/core/logical_plan_test.go index 2af974ba048c8..ee92ee927a0f8 100644 --- a/planner/core/logical_plan_test.go +++ b/planner/core/logical_plan_test.go @@ -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", + 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", }, {