From e4994a2b660f82e1b3e8b1f611c4d8e1e040d2d0 Mon Sep 17 00:00:00 2001 From: Jian Zhang Date: Thu, 22 Nov 2018 21:08:20 +0800 Subject: [PATCH 1/2] planner/core, expression: don't pushdown filters contains set or get var --- expression/chunk_executor.go | 8 +-- expression/integration_test.go | 73 ++++++++++++++++++++++++ planner/core/rule_predicate_push_down.go | 30 ++++++++-- 3 files changed, 103 insertions(+), 8 deletions(-) diff --git a/expression/chunk_executor.go b/expression/chunk_executor.go index 5a02dc88ce94a..61be58c1a71fe 100644 --- a/expression/chunk_executor.go +++ b/expression/chunk_executor.go @@ -27,15 +27,15 @@ import ( // Vectorizable checks whether a list of expressions can employ vectorized execution. func Vectorizable(exprs []Expression) bool { for _, expr := range exprs { - if hasUnVectorizableFunc(expr) { + if HasUnVectorizableFunc(expr) { return false } } return true } -// hasUnVectorizableFunc checks whether an expression contains functions that can not utilize the vectorized execution. -func hasUnVectorizableFunc(expr Expression) bool { +// HasUnVectorizableFunc checks whether an expression contains functions that can not utilize the vectorized execution. +func HasUnVectorizableFunc(expr Expression) bool { scalaFunc, ok := expr.(*ScalarFunction) if !ok { return false @@ -47,7 +47,7 @@ func hasUnVectorizableFunc(expr Expression) bool { return true } for _, arg := range scalaFunc.GetArgs() { - if hasUnVectorizableFunc(arg) { + if HasUnVectorizableFunc(arg) { return true } } diff --git a/expression/integration_test.go b/expression/integration_test.go index de3624240cb18..d69982117f27e 100644 --- a/expression/integration_test.go +++ b/expression/integration_test.go @@ -3692,3 +3692,76 @@ func (s *testIntegrationSuite) TestValuesInNonInsertStmt(c *C) { res := tk.MustQuery(`select values(a), values(b), values(c), values(d), values(e), values(f), values(g) from t;`) res.Check(testkit.Rows(` `)) } + +func (s *testIntegrationSuite) TestUserVarMockWindFunc(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec(`use test;`) + tk.MustExec(`drop table if exists t;`) + tk.MustExec(`create table t (a int, b varchar (20), c varchar (20));`) + tk.MustExec(`insert into t values + (1,'key1-value1','insert_order1'), + (1,'key1-value2','insert_order2'), + (1,'key1-value3','insert_order3'), + (1,'key1-value4','insert_order4'), + (1,'key1-value5','insert_order5'), + (1,'key1-value6','insert_order6'), + (2,'key2-value1','insert_order1'), + (2,'key2-value2','insert_order2'), + (2,'key2-value3','insert_order3'), + (2,'key2-value4','insert_order4'), + (2,'key2-value5','insert_order5'), + (2,'key2-value6','insert_order6'), + (3,'key3-value1','insert_order1'), + (3,'key3-value2','insert_order2'), + (3,'key3-value3','insert_order3'), + (3,'key3-value4','insert_order4'), + (3,'key3-value5','insert_order5'), + (3,'key3-value6','insert_order6'); + `) + tk.MustExec(`SET @LAST_VAL := NULL;`) + tk.MustExec(`SET @ROW_NUM := 0;`) + + tk.MustQuery(`select * from ( + SELECT a, + @ROW_NUM := IF(a = @LAST_VAL, @ROW_NUM + 1, 1) AS ROW_NUM, + @LAST_VAL := a AS LAST_VAL, + b, + c + FROM (select * from t where a in (1, 2, 3) ORDER BY a, c) t1 + ) t2 + where t2.ROW_NUM < 2; + `).Check(testkit.Rows( + `1 1 1 key1-value1 insert_order1`, + `2 1 2 key2-value1 insert_order1`, + `3 1 3 key3-value1 insert_order1`, + )) + + tk.MustQuery(`select * from ( + SELECT a, + @ROW_NUM := IF(a = @LAST_VAL, @ROW_NUM + 1, 1) AS ROW_NUM, + @LAST_VAL := a AS LAST_VAL, + b, + c + FROM (select * from t where a in (1, 2, 3) ORDER BY a, c) t1 + ) t2; + `).Check(testkit.Rows( + `1 1 1 key1-value1 insert_order1`, + `1 2 1 key1-value2 insert_order2`, + `1 3 1 key1-value3 insert_order3`, + `1 4 1 key1-value4 insert_order4`, + `1 5 1 key1-value5 insert_order5`, + `1 6 1 key1-value6 insert_order6`, + `2 1 2 key2-value1 insert_order1`, + `2 2 2 key2-value2 insert_order2`, + `2 3 2 key2-value3 insert_order3`, + `2 4 2 key2-value4 insert_order4`, + `2 5 2 key2-value5 insert_order5`, + `2 6 2 key2-value6 insert_order6`, + `3 1 3 key3-value1 insert_order1`, + `3 2 3 key3-value2 insert_order2`, + `3 3 3 key3-value3 insert_order3`, + `3 4 3 key3-value4 insert_order4`, + `3 5 3 key3-value5 insert_order5`, + `3 6 3 key3-value6 insert_order6`, + )) +} diff --git a/planner/core/rule_predicate_push_down.go b/planner/core/rule_predicate_push_down.go index dd95c5217c643..17c5d379d4e19 100644 --- a/planner/core/rule_predicate_push_down.go +++ b/planner/core/rule_predicate_push_down.go @@ -56,9 +56,24 @@ func (p *baseLogicalPlan) PredicatePushDown(predicates []expression.Expression) return nil, p.self } +func splitSetGetVarFunc(filters []expression.Expression) ([]expression.Expression, []expression.Expression) { + canBePushDown := make([]expression.Expression, 0, len(filters)) + canNotBePushDown := make([]expression.Expression, 0, len(filters)) + for _, expr := range filters { + if expression.HasUnVectorizableFunc(expr) { + canNotBePushDown = append(canNotBePushDown, expr) + } else { + canBePushDown = append(canBePushDown, expr) + } + } + return canBePushDown, canNotBePushDown +} + // PredicatePushDown implements LogicalPlan PredicatePushDown interface. func (p *LogicalSelection) PredicatePushDown(predicates []expression.Expression) ([]expression.Expression, LogicalPlan) { - retConditions, child := p.children[0].PredicatePushDown(append(p.Conditions, predicates...)) + canBePushDown, canNotBePushDown := splitSetGetVarFunc(p.Conditions) + retConditions, child := p.children[0].PredicatePushDown(append(canBePushDown, predicates...)) + retConditions = append(retConditions, canNotBePushDown...) if len(retConditions) > 0 { p.Conditions = expression.PropagateConstant(p.ctx, retConditions) // Return table dual when filter is constant false or null. @@ -306,11 +321,18 @@ func isNullRejected(ctx sessionctx.Context, schema *expression.Schema, expr expr // PredicatePushDown implements LogicalPlan PredicatePushDown interface. func (p *LogicalProjection) PredicatePushDown(predicates []expression.Expression) (ret []expression.Expression, retPlan LogicalPlan) { - var push = make([]expression.Expression, 0, p.Schema().Len()) + canBePushed := make([]expression.Expression, 0, len(predicates)) + canNotBePushed := make([]expression.Expression, 0, len(predicates)) for _, cond := range predicates { - push = append(push, expression.ColumnSubstitute(cond, p.Schema(), p.Exprs)) + newFilter := expression.ColumnSubstitute(cond, p.Schema(), p.Exprs) + if !expression.HasUnVectorizableFunc(newFilter) { + canBePushed = append(canBePushed, expression.ColumnSubstitute(cond, p.Schema(), p.Exprs)) + } else { + canNotBePushed = append(canNotBePushed, cond) + } } - return p.baseLogicalPlan.PredicatePushDown(push) + remained, child := p.baseLogicalPlan.PredicatePushDown(canBePushed) + return append(remained, canNotBePushed...), child } // PredicatePushDown implements LogicalPlan PredicatePushDown interface. From 6ab39ee745b37629a3922d167031ccb7acf6af53 Mon Sep 17 00:00:00 2001 From: Jian Zhang Date: Sat, 24 Nov 2018 11:22:06 +0800 Subject: [PATCH 2/2] address comment --- expression/chunk_executor.go | 8 ++++---- planner/core/rule_predicate_push_down.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/expression/chunk_executor.go b/expression/chunk_executor.go index 61be58c1a71fe..7434ccda38963 100644 --- a/expression/chunk_executor.go +++ b/expression/chunk_executor.go @@ -27,15 +27,15 @@ import ( // Vectorizable checks whether a list of expressions can employ vectorized execution. func Vectorizable(exprs []Expression) bool { for _, expr := range exprs { - if HasUnVectorizableFunc(expr) { + if HasGetSetVarFunc(expr) { return false } } return true } -// HasUnVectorizableFunc checks whether an expression contains functions that can not utilize the vectorized execution. -func HasUnVectorizableFunc(expr Expression) bool { +// HasGetSetVarFunc checks whether an expression contains SetVar/GetVar function. +func HasGetSetVarFunc(expr Expression) bool { scalaFunc, ok := expr.(*ScalarFunction) if !ok { return false @@ -47,7 +47,7 @@ func HasUnVectorizableFunc(expr Expression) bool { return true } for _, arg := range scalaFunc.GetArgs() { - if HasUnVectorizableFunc(arg) { + if HasGetSetVarFunc(arg) { return true } } diff --git a/planner/core/rule_predicate_push_down.go b/planner/core/rule_predicate_push_down.go index 17c5d379d4e19..86a9e1f14f1f1 100644 --- a/planner/core/rule_predicate_push_down.go +++ b/planner/core/rule_predicate_push_down.go @@ -60,7 +60,7 @@ func splitSetGetVarFunc(filters []expression.Expression) ([]expression.Expressio canBePushDown := make([]expression.Expression, 0, len(filters)) canNotBePushDown := make([]expression.Expression, 0, len(filters)) for _, expr := range filters { - if expression.HasUnVectorizableFunc(expr) { + if expression.HasGetSetVarFunc(expr) { canNotBePushDown = append(canNotBePushDown, expr) } else { canBePushDown = append(canBePushDown, expr) @@ -325,7 +325,7 @@ func (p *LogicalProjection) PredicatePushDown(predicates []expression.Expression canNotBePushed := make([]expression.Expression, 0, len(predicates)) for _, cond := range predicates { newFilter := expression.ColumnSubstitute(cond, p.Schema(), p.Exprs) - if !expression.HasUnVectorizableFunc(newFilter) { + if !expression.HasGetSetVarFunc(newFilter) { canBePushed = append(canBePushed, expression.ColumnSubstitute(cond, p.Schema(), p.Exprs)) } else { canNotBePushed = append(canNotBePushed, cond)