Skip to content

Commit

Permalink
planner: don't push down some conditions to projections (#37992) (#38596
Browse files Browse the repository at this point in the history
)

ref #35623
  • Loading branch information
Reminiscent authored Oct 21, 2022
1 parent 5263a0a commit e8eeccc
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 9 deletions.
23 changes: 18 additions & 5 deletions expression/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,10 @@ func ExtractCorColumns(expr Expression) (cols []*CorrelatedColumn) {
// It's often observed that the pattern of the caller like this:
//
// cols := ExtractColumns(...)
// for _, col := range cols {
// if xxx(col) {...}
// }
//
// for _, col := range cols {
// if xxx(col) {...}
// }
//
// Provide an additional filter argument, this can be done in one step.
// To avoid allocation for cols that not need.
Expand Down Expand Up @@ -379,6 +380,17 @@ func setExprColumnInOperand(expr Expression) Expression {
return expr
}

// ColumnSubstitute4PPD substitutes the columns in filter to expressions in select fields.
// Only used for predicate push down to projection. Some columns can not be substituted for some reasons.
// So we should return the bool value to indicate some expressions can not be pushed down.
// e.g. CREATE TABLE t3(c0 INT, primary key(c0));
// SELECT v2.c0 FROM (select 1 as c0 from t3) v2 WHERE (v2.c0)like(True);
// The cond `(v2.c0)like(True)` can not be substituted when the new collation enable. So we shouldn't push the cond down to the projection.
func ColumnSubstitute4PPD(expr Expression, schema *Schema, newExprs []Expression) (bool, Expression) {
substituted, resExpr := ColumnSubstituteImpl(expr, schema, newExprs)
return substituted, resExpr
}

// ColumnSubstitute substitutes the columns in filter to expressions in select fields.
// e.g. select * from (select b as a from t) k where a < 10 => select * from (select b as a from t where b < 10) k.
func ColumnSubstitute(expr Expression, schema *Schema, newExprs []Expression) Expression {
Expand Down Expand Up @@ -702,8 +714,9 @@ func ContainOuterNot(expr Expression) bool {
// Input `not` means whether there is `not` outside `expr`
//
// eg.
// not(0+(t.a == 1 and t.b == 2)) returns true
// not(t.a) and not(t.b) returns false
//
// not(0+(t.a == 1 and t.b == 2)) returns true
// not(t.a) and not(t.b) returns false
func containOuterNot(expr Expression, not bool) bool {
if f, ok := expr.(*ScalarFunction); ok {
switch f.FuncName.L {
Expand Down
5 changes: 3 additions & 2 deletions planner/cascades/transformation_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -549,8 +549,9 @@ func (r *PushSelDownProjection) OnTransform(old *memo.ExprIter) (newExprs []*mem
canBePushed := make([]expression.Expression, 0, len(sel.Conditions))
canNotBePushed := make([]expression.Expression, 0, len(sel.Conditions))
for _, cond := range sel.Conditions {
if !expression.HasGetSetVarFunc(cond) {
canBePushed = append(canBePushed, expression.ColumnSubstitute(cond, projSchema, proj.Exprs))
substituted, newFilter := expression.ColumnSubstitute4PPD(cond, projSchema, proj.Exprs)
if substituted && !expression.HasGetSetVarFunc(newFilter) {
canBePushed = append(canBePushed, newFilter)
} else {
canNotBePushed = append(canNotBePushed, cond)
}
Expand Down
27 changes: 27 additions & 0 deletions planner/core/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,33 @@ func TestIssue22828(t *testing.T) {
tk.MustGetErrMsg(`select group_concat((select concat(c,group_concat(c)) FROM t where xxx=xxx)) FROM t;`, "[planner:1054]Unknown column 'xxx' in 'where clause'")
}

func TestIssue35623(t *testing.T) {
store, clean := testkit.CreateMockStore(t)
defer clean()
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec(`drop table if exists t1;`)
tk.MustExec(`drop view if exists v1;`)
tk.MustExec(`CREATE TABLE t1(c0 INT UNIQUE);`)
tk.MustExec("CREATE definer='root'@'localhost' VIEW v1(c0) AS SELECT 1 FROM t1;")
err := tk.ExecToErr("SELECT v1.c0 FROM v1 WHERE (true)LIKE(v1.c0);")
require.NoError(t, err)

err = tk.ExecToErr("SELECT v2.c0 FROM (select 1 as c0 from t1) v2 WHERE (v2.c0)like(True);")
require.NoError(t, err)
}

func TestIssue37971(t *testing.T) {
store, clean := testkit.CreateMockStore(t)
defer clean()
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec(`drop table if exists t3;`)
tk.MustExec(`CREATE TABLE t3(c0 INT, primary key(c0));`)
err := tk.ExecToErr("SELECT v2.c0 FROM (select 1 as c0 from t3) v2 WHERE (v2.c0)like(True);")
require.NoError(t, err)
}

func TestJoinNotNullFlag(t *testing.T) {
store, clean := testkit.CreateMockStore(t)
defer clean()
Expand Down
4 changes: 2 additions & 2 deletions planner/core/rule_predicate_push_down.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,8 +418,8 @@ func (p *LogicalProjection) PredicatePushDown(predicates []expression.Expression
}
}
for _, cond := range predicates {
newFilter := expression.ColumnSubstitute(cond, p.Schema(), p.Exprs)
if !expression.HasGetSetVarFunc(newFilter) {
substituted, newFilter := expression.ColumnSubstitute4PPD(cond, p.Schema(), p.Exprs)
if substituted && !expression.HasGetSetVarFunc(newFilter) {
canBePushed = append(canBePushed, newFilter)
} else {
canNotBePushed = append(canNotBePushed, cond)
Expand Down

0 comments on commit e8eeccc

Please sign in to comment.