From 4653f815bf22db5c94106641ffcc20dd7cc91636 Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sat, 8 Sep 2018 09:59:48 +0800 Subject: [PATCH 01/18] expression: try to fold constant for expression with null parameter --- expression/constant_fold.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/expression/constant_fold.go b/expression/constant_fold.go index f961a3c5d6978..73baa6e4f68f3 100644 --- a/expression/constant_fold.go +++ b/expression/constant_fold.go @@ -86,20 +86,26 @@ func foldConstant(expr Expression) (Expression, bool) { } args := x.GetArgs() - canFold := true + allConstArg := true + hasNullArg := false isDeferredConst := false for i := 0; i < len(args); i++ { foldedArg, isDeferred := foldConstant(args[i]) x.GetArgs()[i] = foldedArg - _, conOK := foldedArg.(*Constant) - if !conOK { - canFold = false + con, conOK := foldedArg.(*Constant) + if conOK { + if con.Value.IsNull() { + hasNullArg = true + } + } else { + allConstArg = false } isDeferredConst = isDeferredConst || isDeferred } - if !canFold { + if !allConstArg && !hasNullArg { return expr, isDeferredConst } + // Convert Column and ScalarFunction to DummyConstant value, err := x.Eval(chunk.Row{}) if err != nil { log.Warnf("fold constant %s: %s", x.ExplainInfo(), err.Error()) From b23e33df5450f3151ee3c20655669e81bed13634 Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sat, 8 Sep 2018 11:04:31 +0800 Subject: [PATCH 02/18] substitute non-Constant arg to One --- expression/constant_fold.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/expression/constant_fold.go b/expression/constant_fold.go index 73baa6e4f68f3..92956566f20e9 100644 --- a/expression/constant_fold.go +++ b/expression/constant_fold.go @@ -86,7 +86,7 @@ func foldConstant(expr Expression) (Expression, bool) { } args := x.GetArgs() - allConstArg := true + nonConstArgIdx := make([]int, 0, len(args)) hasNullArg := false isDeferredConst := false for i := 0; i < len(args); i++ { @@ -98,14 +98,24 @@ func foldConstant(expr Expression) (Expression, bool) { hasNullArg = true } } else { - allConstArg = false + nonConstArgIdx = append(nonConstArgIdx, i) } isDeferredConst = isDeferredConst || isDeferred } - if !allConstArg && !hasNullArg { - return expr, isDeferredConst + if len(nonConstArgIdx) > 0 { + if !hasNullArg { + return expr, isDeferredConst + } + dummyScalarFunc, _ := x.Clone().(*ScalarFunction) + for _, idx := range nonConstArgIdx { + dummyScalarFunc.GetArgs()[idx] = One + } + value, err := dummyScalarFunc.Eval(chunk.Row{}) + if err != nil || !value.IsNull() { + return expr, isDeferredConst + } + return &Constant{Value: value, RetType: x.RetType}, false } - // Convert Column and ScalarFunction to DummyConstant value, err := x.Eval(chunk.Row{}) if err != nil { log.Warnf("fold constant %s: %s", x.ExplainInfo(), err.Error()) From 26cc55370a88b81a21095165d0490c7245644c2b Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sat, 8 Sep 2018 13:48:41 +0800 Subject: [PATCH 03/18] add PruneColumns for LogicalTableDual --- plan/rule_column_pruning.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/plan/rule_column_pruning.go b/plan/rule_column_pruning.go index cd3e38adfed7e..845195784f0cb 100644 --- a/plan/rule_column_pruning.go +++ b/plan/rule_column_pruning.go @@ -179,6 +179,21 @@ func (p *LogicalTableDual) PruneColumns(parentUsedCols []*expression.Column) { } } +// PruneColumns implements LogicalPlan interface. +func (p *LogicalTableDual) PruneColumns(parentUsedCols []*expression.Column) { + used := getUsedList(parentUsedCols, p.schema) + for i := len(used) - 1; i >= 0; i-- { + if !used[i] { + p.schema.Columns = append(p.schema.Columns[:i], p.schema.Columns[i+1:]...) + } + } + for k, cols := range p.schema.TblID2Handle { + if p.schema.ColumnIndex(cols[0]) == -1 { + delete(p.schema.TblID2Handle, k) + } + } +} + func (p *LogicalJoin) extractUsedCols(parentUsedCols []*expression.Column) (leftCols []*expression.Column, rightCols []*expression.Column) { for _, eqCond := range p.EqualConditions { parentUsedCols = append(parentUsedCols, expression.ExtractColumns(eqCond)...) From 6260ca036cbfba089bc2a493343d0e96d9b667eb Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sat, 8 Sep 2018 18:05:40 +0800 Subject: [PATCH 04/18] disable TestPreparedNullParam for plan cache enabled setting --- executor/prepared_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/executor/prepared_test.go b/executor/prepared_test.go index feb0e10829bab..48e64393734dc 100644 --- a/executor/prepared_test.go +++ b/executor/prepared_test.go @@ -229,7 +229,10 @@ func (s *testSuite) TestPreparedNullParam(c *C) { plan.SetPreparedPlanCache(orgEnable) plan.PreparedPlanCacheCapacity = orgCapacity }() - flags := []bool{false, true} + // Disable this test for plan cache enabled setting temporarily + // Reopen it when issue #7579 is fixed + // flags := []bool{false, true} + flags := []bool{false} for _, flag := range flags { plan.SetPreparedPlanCache(flag) plan.PreparedPlanCacheCapacity = 100 From ce4033ede896720e78c2887fc5699088e23c0209 Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sun, 9 Sep 2018 02:35:59 +0800 Subject: [PATCH 05/18] push down not first before foldConstant only do null fold in null reject check --- expression/constant_fold.go | 4 +++- plan/rule_predicate_push_down.go | 4 +++- sessionctx/stmtctx/stmtctx.go | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/expression/constant_fold.go b/expression/constant_fold.go index 92956566f20e9..610c83f624c10 100644 --- a/expression/constant_fold.go +++ b/expression/constant_fold.go @@ -31,6 +31,7 @@ func init() { // FoldConstant does constant folding optimization on an expression excluding deferred ones. func FoldConstant(expr Expression) Expression { + expr = PushDownNot(nil, expr, false) e, _ := foldConstant(expr) return e } @@ -86,6 +87,7 @@ func foldConstant(expr Expression) (Expression, bool) { } args := x.GetArgs() + sc := x.GetCtx().GetSessionVars().StmtCtx nonConstArgIdx := make([]int, 0, len(args)) hasNullArg := false isDeferredConst := false @@ -103,7 +105,7 @@ func foldConstant(expr Expression) (Expression, bool) { isDeferredConst = isDeferredConst || isDeferred } if len(nonConstArgIdx) > 0 { - if !hasNullArg { + if !hasNullArg || !sc.InNullRejectCheck { return expr, isDeferredConst } dummyScalarFunc, _ := x.Clone().(*ScalarFunction) diff --git a/plan/rule_predicate_push_down.go b/plan/rule_predicate_push_down.go index 16f14b8f5c363..3ce96ae89e911 100644 --- a/plan/rule_predicate_push_down.go +++ b/plan/rule_predicate_push_down.go @@ -260,12 +260,14 @@ func simplifyOuterJoin(p *LogicalJoin, predicates []expression.Expression) { // If it is a conjunction containing a null-rejected condition as a conjunct. // If it is a disjunction of null-rejected conditions. func isNullRejected(ctx sessionctx.Context, schema *expression.Schema, expr expression.Expression) bool { + sc := ctx.GetSessionVars().StmtCtx + sc.InNullRejectCheck = true result := expression.EvaluateExprWithNull(ctx, schema, expr) + sc.InNullRejectCheck = false x, ok := result.(*expression.Constant) if !ok { return false } - sc := ctx.GetSessionVars().StmtCtx if x.Value.IsNull() { return true } else if isTrue, err := x.Value.ToBool(sc); err != nil || isTrue == 0 { diff --git a/sessionctx/stmtctx/stmtctx.go b/sessionctx/stmtctx/stmtctx.go index 631e859940d95..2fc9adfd49712 100644 --- a/sessionctx/stmtctx/stmtctx.go +++ b/sessionctx/stmtctx/stmtctx.go @@ -60,6 +60,7 @@ type StatementContext struct { UseCache bool PadCharToFullLength bool BatchCheck bool + InNullRejectCheck bool // mu struct holds variables that change during execution. mu struct { From 615d985f48e0b1a8d95fd03bd47f33ee97f7518f Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sun, 9 Sep 2018 02:43:57 +0800 Subject: [PATCH 06/18] revert PruneColumns of LogicalTableDual, this would be in another PR plan cache test should pass now, reopen it --- executor/prepared_test.go | 5 +---- plan/cbo_test.go | 3 ++- plan/physical_plan_test.go | 4 ++-- plan/rule_column_pruning.go | 15 --------------- 4 files changed, 5 insertions(+), 22 deletions(-) diff --git a/executor/prepared_test.go b/executor/prepared_test.go index 48e64393734dc..feb0e10829bab 100644 --- a/executor/prepared_test.go +++ b/executor/prepared_test.go @@ -229,10 +229,7 @@ func (s *testSuite) TestPreparedNullParam(c *C) { plan.SetPreparedPlanCache(orgEnable) plan.PreparedPlanCacheCapacity = orgCapacity }() - // Disable this test for plan cache enabled setting temporarily - // Reopen it when issue #7579 is fixed - // flags := []bool{false, true} - flags := []bool{false} + flags := []bool{false, true} for _, flag := range flags { plan.SetPreparedPlanCache(flag) plan.PreparedPlanCacheCapacity = 100 diff --git a/plan/cbo_test.go b/plan/cbo_test.go index 2901a045e24e5..9c2f4cf609aec 100644 --- a/plan/cbo_test.go +++ b/plan/cbo_test.go @@ -144,7 +144,8 @@ func (s *testAnalyzeSuite) TestTableDual(c *C) { c.Assert(h.Update(dom.InfoSchema()), IsNil) testKit.MustQuery(`explain select * from t where 1 = 0`).Check(testkit.Rows( - `TableDual_6 0.00 root rows:0`, + `Projection_5 0.00 root test.t.a`, + `└─TableDual_6 0.00 root rows:0`, )) testKit.MustQuery(`explain select * from t where 1 = 1 limit 0`).Check(testkit.Rows( diff --git a/plan/physical_plan_test.go b/plan/physical_plan_test.go index 3f2aae7d03eb0..ce7227fa33081 100644 --- a/plan/physical_plan_test.go +++ b/plan/physical_plan_test.go @@ -1124,7 +1124,7 @@ func (s *testPlanSuite) TestRefine(c *C) { }, { sql: `select a from t where c = 1.9 and d > 3`, - best: "Dual", + best: "Dual->Projection", }, { sql: `select a from t where c < 1.1`, @@ -1144,7 +1144,7 @@ func (s *testPlanSuite) TestRefine(c *C) { }, { sql: `select a from t where c = 123456789098765432101234`, - best: "Dual", + best: "Dual->Projection", }, { sql: `select a from t where c = 'hanfei'`, diff --git a/plan/rule_column_pruning.go b/plan/rule_column_pruning.go index 845195784f0cb..cd3e38adfed7e 100644 --- a/plan/rule_column_pruning.go +++ b/plan/rule_column_pruning.go @@ -179,21 +179,6 @@ func (p *LogicalTableDual) PruneColumns(parentUsedCols []*expression.Column) { } } -// PruneColumns implements LogicalPlan interface. -func (p *LogicalTableDual) PruneColumns(parentUsedCols []*expression.Column) { - used := getUsedList(parentUsedCols, p.schema) - for i := len(used) - 1; i >= 0; i-- { - if !used[i] { - p.schema.Columns = append(p.schema.Columns[:i], p.schema.Columns[i+1:]...) - } - } - for k, cols := range p.schema.TblID2Handle { - if p.schema.ColumnIndex(cols[0]) == -1 { - delete(p.schema.TblID2Handle, k) - } - } -} - func (p *LogicalJoin) extractUsedCols(parentUsedCols []*expression.Column) (leftCols []*expression.Column, rightCols []*expression.Column) { for _, eqCond := range p.EqualConditions { parentUsedCols = append(parentUsedCols, expression.ExtractColumns(eqCond)...) From 6effd4f5e0c6bd01c9b1e03fb5a2a605202f56af Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sun, 9 Sep 2018 03:01:53 +0800 Subject: [PATCH 07/18] move PushDownNot to isNullRejected to avoid stack overflow --- expression/constant_fold.go | 1 - plan/rule_predicate_push_down.go | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/expression/constant_fold.go b/expression/constant_fold.go index 610c83f624c10..c0d18bea3aea4 100644 --- a/expression/constant_fold.go +++ b/expression/constant_fold.go @@ -31,7 +31,6 @@ func init() { // FoldConstant does constant folding optimization on an expression excluding deferred ones. func FoldConstant(expr Expression) Expression { - expr = PushDownNot(nil, expr, false) e, _ := foldConstant(expr) return e } diff --git a/plan/rule_predicate_push_down.go b/plan/rule_predicate_push_down.go index 3ce96ae89e911..8e9f74e9ac6b3 100644 --- a/plan/rule_predicate_push_down.go +++ b/plan/rule_predicate_push_down.go @@ -260,6 +260,7 @@ func simplifyOuterJoin(p *LogicalJoin, predicates []expression.Expression) { // If it is a conjunction containing a null-rejected condition as a conjunct. // If it is a disjunction of null-rejected conditions. func isNullRejected(ctx sessionctx.Context, schema *expression.Schema, expr expression.Expression) bool { + expr = expression.PushDownNot(nil, expr, false) sc := ctx.GetSessionVars().StmtCtx sc.InNullRejectCheck = true result := expression.EvaluateExprWithNull(ctx, schema, expr) From fbef1a3b4cfe8d2c71525e8404c45e826e608fe7 Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sun, 9 Sep 2018 03:56:35 +0800 Subject: [PATCH 08/18] update tpch explain test because left outer semi join is converted to inner join now --- cmd/explaintest/r/tpch.result | 92 +++++++++++++++++------------------ 1 file changed, 45 insertions(+), 47 deletions(-) diff --git a/cmd/explaintest/r/tpch.result b/cmd/explaintest/r/tpch.result index f0e024bfd0a3a..1efe1d727b6ec 100644 --- a/cmd/explaintest/r/tpch.result +++ b/cmd/explaintest/r/tpch.result @@ -184,38 +184,37 @@ limit 100; id count task operator info Projection_34 100.00 root tpch.supplier.s_acctbal, tpch.supplier.s_name, tpch.nation.n_name, tpch.part.p_partkey, tpch.part.p_mfgr, tpch.supplier.s_address, tpch.supplier.s_phone, tpch.supplier.s_comment └─TopN_37 100.00 root tpch.supplier.s_acctbal:desc, tpch.nation.n_name:asc, tpch.supplier.s_name:asc, tpch.part.p_partkey:asc, offset:0, count:100 - └─Selection_38 100087.54 root eq(tpch.partsupp.ps_supplycost, min(ps_supplycost)) - └─HashLeftJoin_39 125109.42 root left outer join, inner:HashAgg_88, equal:[eq(tpch.part.p_partkey, tpch.partsupp.ps_partkey)] - ├─HashLeftJoin_44 125109.42 root inner join, inner:TableReader_85, equal:[eq(tpch.nation.n_regionkey, tpch.region.r_regionkey)] - │ ├─HashLeftJoin_49 625547.12 root inner join, inner:TableReader_82, equal:[eq(tpch.supplier.s_nationkey, tpch.nation.n_nationkey)] - │ │ ├─IndexJoin_53 625547.12 root inner join, inner:TableReader_52, outer key:tpch.partsupp.ps_suppkey, inner key:tpch.supplier.s_suppkey - │ │ │ ├─IndexJoin_60 625547.12 root inner join, inner:IndexLookUp_59, outer key:tpch.part.p_partkey, inner key:tpch.partsupp.ps_partkey - │ │ │ │ ├─TableReader_76 155496.00 root data:Selection_75 - │ │ │ │ │ └─Selection_75 155496.00 cop eq(tpch.part.p_size, 30), like(tpch.part.p_type, "%STEEL", 92) - │ │ │ │ │ └─TableScan_74 10000000.00 cop table:part, range:[-inf,+inf], keep order:false - │ │ │ │ └─IndexLookUp_59 1.00 root - │ │ │ │ ├─IndexScan_57 1.00 cop table:partsupp, index:PS_PARTKEY, PS_SUPPKEY, range: decided by [tpch.part.p_partkey], keep order:false - │ │ │ │ └─TableScan_58 1.00 cop table:partsupp, keep order:false - │ │ │ └─TableReader_52 1.00 root data:TableScan_51 - │ │ │ └─TableScan_51 1.00 cop table:supplier, range: decided by [tpch.partsupp.ps_suppkey], keep order:false - │ │ └─TableReader_82 25.00 root data:TableScan_81 - │ │ └─TableScan_81 25.00 cop table:nation, range:[-inf,+inf], keep order:false - │ └─TableReader_85 1.00 root data:Selection_84 - │ └─Selection_84 1.00 cop eq(tpch.region.r_name, "ASIA") - │ └─TableScan_83 5.00 cop table:region, range:[-inf,+inf], keep order:false - └─HashAgg_88 8155010.44 root group by:tpch.partsupp.ps_partkey, funcs:min(tpch.partsupp.ps_supplycost), firstrow(tpch.partsupp.ps_partkey) - └─HashRightJoin_92 8155010.44 root inner join, inner:HashRightJoin_94, equal:[eq(tpch.supplier.s_suppkey, tpch.partsupp.ps_suppkey)] - ├─HashRightJoin_94 100000.00 root inner join, inner:HashRightJoin_100, equal:[eq(tpch.nation.n_nationkey, tpch.supplier.s_nationkey)] - │ ├─HashRightJoin_100 5.00 root inner join, inner:TableReader_105, equal:[eq(tpch.region.r_regionkey, tpch.nation.n_regionkey)] - │ │ ├─TableReader_105 1.00 root data:Selection_104 - │ │ │ └─Selection_104 1.00 cop eq(tpch.region.r_name, "ASIA") - │ │ │ └─TableScan_103 5.00 cop table:region, range:[-inf,+inf], keep order:false - │ │ └─TableReader_102 25.00 root data:TableScan_101 - │ │ └─TableScan_101 25.00 cop table:nation, range:[-inf,+inf], keep order:false - │ └─TableReader_107 500000.00 root data:TableScan_106 - │ └─TableScan_106 500000.00 cop table:supplier, range:[-inf,+inf], keep order:false - └─TableReader_109 40000000.00 root data:TableScan_108 - └─TableScan_108 40000000.00 cop table:partsupp, range:[-inf,+inf], keep order:false + └─HashRightJoin_39 125109.42 root inner join, inner:HashLeftJoin_44, equal:[eq(tpch.part.p_partkey, tpch.partsupp.ps_partkey) eq(tpch.partsupp.ps_supplycost, min(ps_supplycost))] + ├─HashLeftJoin_44 125109.42 root inner join, inner:TableReader_85, equal:[eq(tpch.nation.n_regionkey, tpch.region.r_regionkey)] + │ ├─HashLeftJoin_49 625547.12 root inner join, inner:TableReader_82, equal:[eq(tpch.supplier.s_nationkey, tpch.nation.n_nationkey)] + │ │ ├─IndexJoin_53 625547.12 root inner join, inner:TableReader_52, outer key:tpch.partsupp.ps_suppkey, inner key:tpch.supplier.s_suppkey + │ │ │ ├─IndexJoin_60 625547.12 root inner join, inner:IndexLookUp_59, outer key:tpch.part.p_partkey, inner key:tpch.partsupp.ps_partkey + │ │ │ │ ├─TableReader_76 155496.00 root data:Selection_75 + │ │ │ │ │ └─Selection_75 155496.00 cop eq(tpch.part.p_size, 30), like(tpch.part.p_type, "%STEEL", 92) + │ │ │ │ │ └─TableScan_74 10000000.00 cop table:part, range:[-inf,+inf], keep order:false + │ │ │ │ └─IndexLookUp_59 1.00 root + │ │ │ │ ├─IndexScan_57 1.00 cop table:partsupp, index:PS_PARTKEY, PS_SUPPKEY, range: decided by [tpch.part.p_partkey], keep order:false + │ │ │ │ └─TableScan_58 1.00 cop table:partsupp, keep order:false + │ │ │ └─TableReader_52 1.00 root data:TableScan_51 + │ │ │ └─TableScan_51 1.00 cop table:supplier, range: decided by [tpch.partsupp.ps_suppkey], keep order:false + │ │ └─TableReader_82 25.00 root data:TableScan_81 + │ │ └─TableScan_81 25.00 cop table:nation, range:[-inf,+inf], keep order:false + │ └─TableReader_85 1.00 root data:Selection_84 + │ └─Selection_84 1.00 cop eq(tpch.region.r_name, "ASIA") + │ └─TableScan_83 5.00 cop table:region, range:[-inf,+inf], keep order:false + └─HashAgg_88 8155010.44 root group by:tpch.partsupp.ps_partkey, funcs:min(tpch.partsupp.ps_supplycost), firstrow(tpch.partsupp.ps_partkey) + └─HashRightJoin_92 8155010.44 root inner join, inner:HashRightJoin_94, equal:[eq(tpch.supplier.s_suppkey, tpch.partsupp.ps_suppkey)] + ├─HashRightJoin_94 100000.00 root inner join, inner:HashRightJoin_100, equal:[eq(tpch.nation.n_nationkey, tpch.supplier.s_nationkey)] + │ ├─HashRightJoin_100 5.00 root inner join, inner:TableReader_105, equal:[eq(tpch.region.r_regionkey, tpch.nation.n_regionkey)] + │ │ ├─TableReader_105 1.00 root data:Selection_104 + │ │ │ └─Selection_104 1.00 cop eq(tpch.region.r_name, "ASIA") + │ │ │ └─TableScan_103 5.00 cop table:region, range:[-inf,+inf], keep order:false + │ │ └─TableReader_102 25.00 root data:TableScan_101 + │ │ └─TableScan_101 25.00 cop table:nation, range:[-inf,+inf], keep order:false + │ └─TableReader_107 500000.00 root data:TableScan_106 + │ └─TableScan_106 500000.00 cop table:supplier, range:[-inf,+inf], keep order:false + └─TableReader_109 40000000.00 root data:TableScan_108 + └─TableScan_108 40000000.00 cop table:partsupp, range:[-inf,+inf], keep order:false /* Q3 Shipping Priority Query This query retrieves the 10 unshipped orders with the highest value. @@ -965,21 +964,20 @@ where l_partkey = p_partkey ); id count task operator info -Projection_15 1.00 root div(11_col_0, 7.0) -└─StreamAgg_20 1.00 root funcs:sum(tpch.lineitem.l_extendedprice) - └─Projection_43 235019.06 root tpch.lineitem.l_partkey, tpch.lineitem.l_quantity, tpch.lineitem.l_extendedprice, tpch.part.p_partkey, tpch.part.p_brand, tpch.part.p_container, mul(0.2, 7_col_0) - └─Selection_44 235019.06 root lt(tpch.lineitem.l_quantity, mul(0.2, 7_col_0)) - └─HashLeftJoin_45 293773.83 root left outer join, inner:HashAgg_39, equal:[eq(tpch.part.p_partkey, tpch.lineitem.l_partkey)] - ├─HashRightJoin_51 293773.83 root inner join, inner:TableReader_34, equal:[eq(tpch.part.p_partkey, tpch.lineitem.l_partkey)] - │ ├─TableReader_34 9736.49 root data:Selection_33 - │ │ └─Selection_33 9736.49 cop eq(tpch.part.p_brand, "Brand#44"), eq(tpch.part.p_container, "WRAP PKG") - │ │ └─TableScan_32 10000000.00 cop table:part, range:[-inf,+inf], keep order:false - │ └─TableReader_53 300005811.00 root data:TableScan_52 - │ └─TableScan_52 300005811.00 cop table:lineitem, range:[-inf,+inf], keep order:false - └─HashAgg_39 9943040.00 root group by:col_3, funcs:avg(col_0, col_1), firstrow(col_2) - └─TableReader_40 9943040.00 root data:HashAgg_35 - └─HashAgg_35 9943040.00 cop group by:tpch.lineitem.l_partkey, funcs:avg(tpch.lineitem.l_quantity), firstrow(tpch.lineitem.l_partkey) - └─TableScan_38 300005811.00 cop table:lineitem, range:[-inf,+inf], keep order:false +Projection_14 1.00 root div(11_col_0, 7.0) +└─StreamAgg_19 1.00 root funcs:sum(tpch.lineitem.l_extendedprice) + └─Projection_42 293773.83 root tpch.lineitem.l_partkey, tpch.lineitem.l_quantity, tpch.lineitem.l_extendedprice, tpch.part.p_partkey, tpch.part.p_brand, tpch.part.p_container, mul(0.2, 7_col_0) + └─HashRightJoin_44 293773.83 root inner join, inner:HashRightJoin_28, equal:[eq(tpch.part.p_partkey, tpch.lineitem.l_partkey)], other cond:lt(tpch.lineitem.l_quantity, mul(0.2, 7_col_0)) + ├─HashRightJoin_28 293773.83 root inner join, inner:TableReader_33, equal:[eq(tpch.part.p_partkey, tpch.lineitem.l_partkey)] + │ ├─TableReader_33 9736.49 root data:Selection_32 + │ │ └─Selection_32 9736.49 cop eq(tpch.part.p_brand, "Brand#44"), eq(tpch.part.p_container, "WRAP PKG") + │ │ └─TableScan_31 10000000.00 cop table:part, range:[-inf,+inf], keep order:false + │ └─TableReader_30 300005811.00 root data:TableScan_29 + │ └─TableScan_29 300005811.00 cop table:lineitem, range:[-inf,+inf], keep order:false + └─HashAgg_59 9943040.00 root group by:col_3, funcs:avg(col_0, col_1), firstrow(col_2) + └─TableReader_60 9943040.00 root data:HashAgg_56 + └─HashAgg_56 9943040.00 cop group by:tpch.lineitem.l_partkey, funcs:avg(tpch.lineitem.l_quantity), firstrow(tpch.lineitem.l_partkey) + └─TableScan_37 300005811.00 cop table:lineitem, range:[-inf,+inf], keep order:false /* Q18 Large Volume Customer Query The Large Volume Customer Query ranks customers based on their having placed a large quantity order. Large From 0304e8130c9e1160a86cae2a633dbe2272657875 Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sun, 9 Sep 2018 07:57:48 +0800 Subject: [PATCH 09/18] add unit test and explain test --- cmd/explaintest/r/explain_easy.result | 5 +++ cmd/explaintest/t/explain_easy.test | 1 + plan/logical_plan_test.go | 62 +++++++++++++++++++++++---- 3 files changed, 60 insertions(+), 8 deletions(-) diff --git a/cmd/explaintest/r/explain_easy.result b/cmd/explaintest/r/explain_easy.result index b040983ba1114..afebb40dd0a07 100644 --- a/cmd/explaintest/r/explain_easy.result +++ b/cmd/explaintest/r/explain_easy.result @@ -352,6 +352,11 @@ create table t(a bigint primary key); explain select * from t where a = 1 and a = 2; id count task operator info TableDual_5 0.00 root rows:0 +explain select null or a > 1 from t; +id count task operator info +Projection_3 10000.00 root or(NULL, gt(test.t.a, 1)) +└─TableReader_5 10000.00 root data:TableScan_4 + └─TableScan_4 10000.00 cop table:t, range:[-inf,+inf], keep order:false, stats:pseudo drop table if exists ta, tb; create table ta (a varchar(20)); create table tb (a varchar(20)); diff --git a/cmd/explaintest/t/explain_easy.test b/cmd/explaintest/t/explain_easy.test index bb93dac029071..0a2d7ef884389 100644 --- a/cmd/explaintest/t/explain_easy.test +++ b/cmd/explaintest/t/explain_easy.test @@ -70,6 +70,7 @@ explain select * from t where b = 1 and b = 2; drop table if exists t; create table t(a bigint primary key); explain select * from t where a = 1 and a = 2; +explain select null or a > 1 from t; drop table if exists ta, tb; create table ta (a varchar(20)); diff --git a/plan/logical_plan_test.go b/plan/logical_plan_test.go index dfe12c8519bbc..92d52f569b6b4 100644 --- a/plan/logical_plan_test.go +++ b/plan/logical_plan_test.go @@ -286,9 +286,8 @@ func mockContext() sessionctx.Context { func (s *testPlanSuite) TestPredicatePushDown(c *C) { defer testleak.AfterTest(c)() tests := []struct { - sql string - first string - best string + sql string + best string }{ { sql: "select count(*) from t a, t b where a.a = b.a", @@ -567,11 +566,6 @@ func (s *testPlanSuite) TestOuterWherePredicatePushDown(c *C) { }, } for _, ca := range tests { - comment := Commentf("for %s", ca.sql) - stmt, err := s.ParseOneStmt(ca.sql, "", "") - c.Assert(err, IsNil, comment) - p, err := BuildLogicalPlan(s.ctx, stmt, s.is) - c.Assert(err, IsNil, comment) p, err = logicalOptimize(flagPredicatePushDown|flagDecorrelate|flagPrunColumns, p.(LogicalPlan)) c.Assert(err, IsNil, comment) proj, ok := p.(*LogicalProjection) @@ -593,6 +587,58 @@ func (s *testPlanSuite) TestOuterWherePredicatePushDown(c *C) { } } +func (s *testPlanSuite) TestSimplifyOuterJoin(c *C) { + defer testleak.AfterTest(c)() + tests := []struct { + sql string + best string + joinType string + }{ + { + sql: "select * from t t1 left join t t2 on t1.b = t2.b where t1.c > 1 or t2.c > 1;", + best: "Join{DataScan(t1)->DataScan(t2)}(t1.b,t2.b)->Sel([or(gt(t1.c, 1), gt(t2.c, 1))])->Projection", + joinType: "left outer join", + }, + { + sql: "select * from t t1 left join t t2 on t1.b = t2.b where t1.c > 1 and t2.c > 1;", + best: "Join{DataScan(t1)->DataScan(t2)}(t1.b,t2.b)->Projection", + joinType: "inner join", + }, + { + sql: "select * from t t1 left join t t2 on t1.b = t2.b where not (t1.c > 1 or t2.c > 1);", + best: "Join{DataScan(t1)->DataScan(t2)}(t1.b,t2.b)->Projection", + joinType: "inner join", + }, + { + 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)}(t1.b,t2.b)->Sel([not(and(le(t1.c, 1), le(t2.c, 1)))])->Projection", + joinType: "left outer join", + }, + { + sql: "select * from t t1 left join t t2 on t1.b > 1 where t1.c = t2.c;", + best: "Join{DataScan(t1)->DataScan(t2)}(t1.c,t2.c)->Projection", + joinType: "inner join", + }, + } + for _, ca := range tests { + comment := Commentf("for %s", ca.sql) + stmt, err := s.ParseOneStmt(ca.sql, "", "") + c.Assert(err, IsNil, comment) + p, err := BuildLogicalPlan(s.ctx, stmt, s.is) + c.Assert(err, IsNil, comment) + p, err = logicalOptimize(flagPredicatePushDown|flagPrunColumns, p.(LogicalPlan)) + c.Assert(err, IsNil, comment) + c.Assert(ToString(p), Equals, ca.best, comment) + join, ok := p.(LogicalPlan).Children()[0].(*LogicalJoin) + if !ok { + join, ok = p.(LogicalPlan).Children()[0].Children()[0].(*LogicalJoin) + c.Assert(ok, IsTrue, comment) + } + joinType := fmt.Sprintf("%s", join.JoinType.String()) + c.Assert(joinType, Equals, ca.joinType, comment) + } +} + func newPartitionInfoSchema(definitions []model.PartitionDefinition) infoschema.InfoSchema { tableInfo := *MockTable() cols := make([]*model.ColumnInfo, 0, len(tableInfo.Columns)) From 21f75ca6bdf7b35eb5d3cf26427dec1994a8616a Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sun, 9 Sep 2018 10:55:26 +0800 Subject: [PATCH 10/18] resolve conflicts --- plan/cbo_test.go | 3 +-- plan/logical_plan_test.go | 5 +++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/plan/cbo_test.go b/plan/cbo_test.go index 9c2f4cf609aec..2901a045e24e5 100644 --- a/plan/cbo_test.go +++ b/plan/cbo_test.go @@ -144,8 +144,7 @@ func (s *testAnalyzeSuite) TestTableDual(c *C) { c.Assert(h.Update(dom.InfoSchema()), IsNil) testKit.MustQuery(`explain select * from t where 1 = 0`).Check(testkit.Rows( - `Projection_5 0.00 root test.t.a`, - `└─TableDual_6 0.00 root rows:0`, + `TableDual_6 0.00 root rows:0`, )) testKit.MustQuery(`explain select * from t where 1 = 1 limit 0`).Check(testkit.Rows( diff --git a/plan/logical_plan_test.go b/plan/logical_plan_test.go index 92d52f569b6b4..9e246244569ab 100644 --- a/plan/logical_plan_test.go +++ b/plan/logical_plan_test.go @@ -566,6 +566,11 @@ func (s *testPlanSuite) TestOuterWherePredicatePushDown(c *C) { }, } for _, ca := range tests { + comment := Commentf("for %s", ca.sql) + stmt, err := s.ParseOneStmt(ca.sql, "", "") + c.Assert(err, IsNil, comment) + p, err := BuildLogicalPlan(s.ctx, stmt, s.is) + c.Assert(err, IsNil, comment) p, err = logicalOptimize(flagPredicatePushDown|flagDecorrelate|flagPrunColumns, p.(LogicalPlan)) c.Assert(err, IsNil, comment) proj, ok := p.(*LogicalProjection) From be77917e395536b739f476022d6e5a949a2ede7b Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sun, 9 Sep 2018 10:57:33 +0800 Subject: [PATCH 11/18] resolve conflicts --- plan/physical_plan_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plan/physical_plan_test.go b/plan/physical_plan_test.go index ce7227fa33081..3f2aae7d03eb0 100644 --- a/plan/physical_plan_test.go +++ b/plan/physical_plan_test.go @@ -1124,7 +1124,7 @@ func (s *testPlanSuite) TestRefine(c *C) { }, { sql: `select a from t where c = 1.9 and d > 3`, - best: "Dual->Projection", + best: "Dual", }, { sql: `select a from t where c < 1.1`, @@ -1144,7 +1144,7 @@ func (s *testPlanSuite) TestRefine(c *C) { }, { sql: `select a from t where c = 123456789098765432101234`, - best: "Dual->Projection", + best: "Dual", }, { sql: `select a from t where c = 'hanfei'`, From 33190d3f896b765f3823c73c0550860d112ee5b6 Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sun, 9 Sep 2018 11:22:57 +0800 Subject: [PATCH 12/18] update TestOuterWherePredicatePushDown since it would be simplified to inner join now --- plan/logical_plan_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plan/logical_plan_test.go b/plan/logical_plan_test.go index 9e246244569ab..8007946f94c89 100644 --- a/plan/logical_plan_test.go +++ b/plan/logical_plan_test.go @@ -547,8 +547,8 @@ func (s *testPlanSuite) TestOuterWherePredicatePushDown(c *C) { }{ // issue #7628, left join with where condition { - sql: "select * from t as t1 left join t as t2 on t1.b = t2.b where (t1.a=1 and t2.a=1) or (t1.a=2 and t2.a=2)", - sel: "[or(and(eq(t1.a, 1), eq(t2.a, 1)), and(eq(t1.a, 2), eq(t2.a, 2)))]", + sql: "select * from t as t1 left join t as t2 on t1.b = t2.b where (t1.a=1 and t2.a is null) or (t1.a=2 and t2.a=2)", + sel: "[or(and(eq(t1.a, 1), isnull(t2.a)), and(eq(t1.a, 2), eq(t2.a, 2)))]", left: "[or(eq(t1.a, 1), eq(t1.a, 2))]", right: "[]", }, @@ -559,8 +559,8 @@ func (s *testPlanSuite) TestOuterWherePredicatePushDown(c *C) { right: "[]", }, { - sql: "select * from t as t1 left join t as t2 on t1.b = t2.b where (t1.c=1 and ((t1.a=3 and t2.a=3) or (t1.a=4 and t2.a=4))) or (t1.a=2 and t2.a=2)", - sel: "[or(and(eq(t1.c, 1), or(and(eq(t1.a, 3), eq(t2.a, 3)), and(eq(t1.a, 4), eq(t2.a, 4)))), and(eq(t1.a, 2), eq(t2.a, 2)))]", + sql: "select * from t as t1 left join t as t2 on t1.b = t2.b where (t1.c=1 and ((t1.a=3 and t2.a=3) or (t1.a=4 and t2.a=4))) or (t1.a=2 and t2.a is null)", + sel: "[or(and(eq(t1.c, 1), or(and(eq(t1.a, 3), eq(t2.a, 3)), and(eq(t1.a, 4), eq(t2.a, 4)))), and(eq(t1.a, 2), isnull(t2.a)))]", left: "[or(and(eq(t1.c, 1), or(eq(t1.a, 3), eq(t1.a, 4))), eq(t1.a, 2))]", right: "[]", }, From 210f8d5c475467bc9f3aaf790711ca67e156cfc9 Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sun, 9 Sep 2018 12:37:58 +0800 Subject: [PATCH 13/18] use NewFunctionInternal instead of Clone fold null if result is null or false --- expression/constant_fold.go | 30 ++++++++++++++++++++++-------- plan/rule_predicate_push_down.go | 2 +- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/expression/constant_fold.go b/expression/constant_fold.go index c0d18bea3aea4..975b3f754984b 100644 --- a/expression/constant_fold.go +++ b/expression/constant_fold.go @@ -87,35 +87,49 @@ func foldConstant(expr Expression) (Expression, bool) { args := x.GetArgs() sc := x.GetCtx().GetSessionVars().StmtCtx - nonConstArgIdx := make([]int, 0, len(args)) + argIsConst := make([]int, 0, len(args)) hasNullArg := false + allConstArg := true isDeferredConst := false for i := 0; i < len(args); i++ { foldedArg, isDeferred := foldConstant(args[i]) x.GetArgs()[i] = foldedArg con, conOK := foldedArg.(*Constant) if conOK { + argIsConst[i] = true if con.Value.IsNull() { hasNullArg = true } } else { - nonConstArgIdx = append(nonConstArgIdx, i) + allConstArg = false + argIsConst[i] = false } isDeferredConst = isDeferredConst || isDeferred } - if len(nonConstArgIdx) > 0 { + if !allConstArg { if !hasNullArg || !sc.InNullRejectCheck { return expr, isDeferredConst } - dummyScalarFunc, _ := x.Clone().(*ScalarFunction) - for _, idx := range nonConstArgIdx { - dummyScalarFunc.GetArgs()[idx] = One + constArgs := make([]Expression, 0, len(args)) + for i, arg := range args { + if argIsConst[i] { + constArgs[i] = arg + } else { + constArgs[i] = One + } } + dummyScalarFunc := NewFunctionInternal(x.GetCtx(), x.FuncName, x.GetType(), constArgs...) value, err := dummyScalarFunc.Eval(chunk.Row{}) - if err != nil || !value.IsNull() { + if err != nil { return expr, isDeferredConst } - return &Constant{Value: value, RetType: x.RetType}, false + if value.IsNull() { + return &Constant{Value: value, RetType: x.RetType}, false + } + if isTrue, err := value.ToBool(sc); err == nil && isTrue == 0 { + return &Constant{Value: value, RetType: x.RetType}, false + } + return expr, isDeferredConst } value, err := x.Eval(chunk.Row{}) if err != nil { diff --git a/plan/rule_predicate_push_down.go b/plan/rule_predicate_push_down.go index 8e9f74e9ac6b3..5327bf7bf9409 100644 --- a/plan/rule_predicate_push_down.go +++ b/plan/rule_predicate_push_down.go @@ -271,7 +271,7 @@ func isNullRejected(ctx sessionctx.Context, schema *expression.Schema, expr expr } if x.Value.IsNull() { return true - } else if isTrue, err := x.Value.ToBool(sc); err != nil || isTrue == 0 { + } else if isTrue, err := x.Value.ToBool(sc); err == nil && isTrue == 0 { return true } return false From b6825da49348e530da6a9213043f2097ef18364c Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sun, 9 Sep 2018 12:43:58 +0800 Subject: [PATCH 14/18] fix compilation error --- expression/constant_fold.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/expression/constant_fold.go b/expression/constant_fold.go index 975b3f754984b..68874459c860d 100644 --- a/expression/constant_fold.go +++ b/expression/constant_fold.go @@ -87,7 +87,7 @@ func foldConstant(expr Expression) (Expression, bool) { args := x.GetArgs() sc := x.GetCtx().GetSessionVars().StmtCtx - argIsConst := make([]int, 0, len(args)) + argIsConst := make([]bool, 0, len(args)) hasNullArg := false allConstArg := true isDeferredConst := false @@ -118,7 +118,7 @@ func foldConstant(expr Expression) (Expression, bool) { constArgs[i] = One } } - dummyScalarFunc := NewFunctionInternal(x.GetCtx(), x.FuncName, x.GetType(), constArgs...) + dummyScalarFunc := NewFunctionInternal(x.GetCtx(), x.FuncName.L, x.GetType(), constArgs...) value, err := dummyScalarFunc.Eval(chunk.Row{}) if err != nil { return expr, isDeferredConst From 2feffbe8ce2863d881f7019736aa5a59b2241e1b Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sun, 9 Sep 2018 12:53:22 +0800 Subject: [PATCH 15/18] index out of range --- expression/constant_fold.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/expression/constant_fold.go b/expression/constant_fold.go index 68874459c860d..fdfa08612d9d7 100644 --- a/expression/constant_fold.go +++ b/expression/constant_fold.go @@ -87,7 +87,7 @@ func foldConstant(expr Expression) (Expression, bool) { args := x.GetArgs() sc := x.GetCtx().GetSessionVars().StmtCtx - argIsConst := make([]bool, 0, len(args)) + argIsConst := make([]bool, len(args)) hasNullArg := false allConstArg := true isDeferredConst := false From 9a781b5a7956e3ab40436390d076260de0d4b5d6 Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sun, 9 Sep 2018 12:59:28 +0800 Subject: [PATCH 16/18] fix index out of range --- expression/constant_fold.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/expression/constant_fold.go b/expression/constant_fold.go index fdfa08612d9d7..8b6eb8be245d5 100644 --- a/expression/constant_fold.go +++ b/expression/constant_fold.go @@ -110,7 +110,7 @@ func foldConstant(expr Expression) (Expression, bool) { if !hasNullArg || !sc.InNullRejectCheck { return expr, isDeferredConst } - constArgs := make([]Expression, 0, len(args)) + constArgs := make([]Expression, len(args)) for i, arg := range args { if argIsConst[i] { constArgs[i] = arg From 33c620aa533c6b33ccff22fa8d27af8da02ff604 Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sun, 9 Sep 2018 17:00:15 +0800 Subject: [PATCH 17/18] address comments --- expression/constant_fold.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/expression/constant_fold.go b/expression/constant_fold.go index 8b6eb8be245d5..e8de5e5dd04d5 100644 --- a/expression/constant_fold.go +++ b/expression/constant_fold.go @@ -95,15 +95,9 @@ func foldConstant(expr Expression) (Expression, bool) { foldedArg, isDeferred := foldConstant(args[i]) x.GetArgs()[i] = foldedArg con, conOK := foldedArg.(*Constant) - if conOK { - argIsConst[i] = true - if con.Value.IsNull() { - hasNullArg = true - } - } else { - allConstArg = false - argIsConst[i] = false - } + argIsConst[i] = conOK + allConstArg = allConstArg && conOK + hasNullArg = hasNullArg || (conOK && con.Value.IsNull()) isDeferredConst = isDeferredConst || isDeferred } if !allConstArg { From d4a32002333d05e5f9e0d6ea8c3111c7ef03c70f Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Sun, 9 Sep 2018 23:52:53 +0800 Subject: [PATCH 18/18] do not fold null for nulleq function add unit test --- expression/constant_fold.go | 2 +- plan/logical_plan_test.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/expression/constant_fold.go b/expression/constant_fold.go index e8de5e5dd04d5..0553b2c8c0c2a 100644 --- a/expression/constant_fold.go +++ b/expression/constant_fold.go @@ -101,7 +101,7 @@ func foldConstant(expr Expression) (Expression, bool) { isDeferredConst = isDeferredConst || isDeferred } if !allConstArg { - if !hasNullArg || !sc.InNullRejectCheck { + if !hasNullArg || !sc.InNullRejectCheck || x.FuncName.L == ast.NullEQ { return expr, isDeferredConst } constArgs := make([]Expression, len(args)) diff --git a/plan/logical_plan_test.go b/plan/logical_plan_test.go index 4c33e6852d07e..78966a2073185 100644 --- a/plan/logical_plan_test.go +++ b/plan/logical_plan_test.go @@ -624,6 +624,11 @@ func (s *testPlanSuite) TestSimplifyOuterJoin(c *C) { best: "Join{DataScan(t1)->DataScan(t2)}(t1.c,t2.c)->Projection", joinType: "inner join", }, + { + sql: "select * from t t1 left join t t2 on true where t1.b <=> t2.b;", + best: "Join{DataScan(t1)->DataScan(t2)}->Sel([nulleq(t1.b, t2.b)])->Projection", + joinType: "left outer join", + }, } for _, ca := range tests { comment := Commentf("for %s", ca.sql)