Skip to content

Commit

Permalink
plan: forbid limit to push across selection. (#2793)
Browse files Browse the repository at this point in the history
  • Loading branch information
hanfei1991 authored and coocood committed Mar 8, 2017
1 parent 517e32b commit 5a377ad
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 15 deletions.
5 changes: 1 addition & 4 deletions plan/match_property.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,10 +231,7 @@ func (p *Selection) matchProperty(prop *requiredProperty, childPlanInfo ...*phys
res.count = uint64(float64(res.count) * selectionFactor)
return res
}
np := *p
np.SetChildren(childPlanInfo[0].p)
count := uint64(float64(childPlanInfo[0].count) * selectionFactor)
return &physicalPlanInfo{p: &np, cost: childPlanInfo[0].cost, count: count}
return childPlanInfo[0]
}

// matchProperty implements PhysicalPlan matchProperty interface.
Expand Down
35 changes: 24 additions & 11 deletions plan/physical_plan_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -807,20 +807,21 @@ func (p *Selection) convert2PhysicalPlan(prop *requiredProperty) (*physicalPlanI
info = infoEnforce
}
}
if ds, ok := p.children[0].(*DataSource); !ok {
info = p.matchProperty(prop, info)
} else {
client := p.ctx.GetClient()
memDB := infoschema.IsMemoryDB(ds.DBName.L)
isDistReq := !memDB && client != nil && client.SupportRequestType(kv.ReqTypeSelect, 0)
if !isDistReq {
info = p.matchProperty(prop, info)
}
}
info = p.matchProperty(prop, info)
p.storePlanInfo(prop, info)
return info, nil
}

func (p *Selection) appendSelToInfo(info *physicalPlanInfo) *physicalPlanInfo {
np := *p
np.SetChildren(info.p)
return &physicalPlanInfo{
p: &np,
cost: info.cost,
count: uint64(float64(info.count) * selectionFactor),
}
}

func (p *Selection) convert2PhysicalPlanPushOrder(prop *requiredProperty) (*physicalPlanInfo, error) {
child := p.children[0].(LogicalPlan)
limit := prop.limit
Expand All @@ -838,7 +839,17 @@ func (p *Selection) convert2PhysicalPlanPushOrder(prop *requiredProperty) (*phys
info = enforceProperty(&requiredProperty{limit: limit}, info)
}
} else {
info = enforceProperty(&requiredProperty{limit: limit}, info)
info = enforceProperty(&requiredProperty{limit: limit}, p.appendSelToInfo(info))
}
} else if ds, ok := p.children[0].(*DataSource); !ok {
// TODO: It's tooooooo tricky here, we will remove all these logic after implementing DAG push down.
info = p.appendSelToInfo(info)
} else {
client := p.ctx.GetClient()
memDB := infoschema.IsMemoryDB(ds.DBName.L)
isDistReq := !memDB && client != nil && client.SupportRequestType(kv.ReqTypeSelect, 0)
if !isDistReq {
info = p.appendSelToInfo(info)
}
}
return info, nil
Expand All @@ -855,6 +866,8 @@ func (p *Selection) convert2PhysicalPlanEnforce(prop *requiredProperty) (*physic
if prop.limit != nil && len(prop.props) > 0 {
if t, ok := info.p.(physicalDistSQLPlan); ok {
t.addTopN(p.ctx, prop)
} else if _, ok := info.p.(*Selection); !ok {
info = p.appendSelToInfo(info)
}
info = enforceProperty(prop, info)
} else if len(prop.props) != 0 {
Expand Down
8 changes: 8 additions & 0 deletions plan/physical_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,14 @@ func (s *testPlanSuite) TestCBO(c *C) {
sql: "select sum(a.b), sum(b.b) from t a join t b on a.c = b.c group by a.d order by a.d",
best: "LeftHashJoin{Table(t)->Table(t)}(a.c,b.c)->HashAgg->Sort->Trim",
},
{
sql: "select * from t t1 left outer join t t2 on true where least(1,2,3,t1.a,t2.b) > 0 order by t2.a limit 10",
best: "LeftHashJoin{Table(t)->Table(t)}->Selection->Sort + Limit(10) + Offset(0)",
},
{
sql: "select * from t t1 left outer join t t2 on true where least(1,2,3,t1.a,t2.b) > 0 limit 10",
best: "LeftHashJoin{Table(t)->Table(t)}->Selection->Limit",
},
{
sql: "select count(*) from t where concat(a,b) = 'abc' group by c",
best: "Index(t.c_d_e)[[<nil>,+inf]]->Selection->StreamAgg",
Expand Down
13 changes: 13 additions & 0 deletions plan/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,19 @@ func (p *requiredProperty) getHashKey() ([]byte, error) {
return bytes, errors.Trace(err)
}

// String implements fmt.Stringer interface. Just for test.
func (p *requiredProperty) String() string {
ret := "Prop{"
for _, colProp := range p.props {
ret += fmt.Sprintf("col: %s, desc %v, ", colProp.col, colProp.desc)
}
ret += fmt.Sprintf("}, Len: %d", p.sortKeyLen)
if p.limit != nil {
ret += fmt.Sprintf(", Limit: %d,%d", p.limit.Offset, p.limit.Count)
}
return ret
}

type physicalPlanInfo struct {
p PhysicalPlan
cost float64
Expand Down

0 comments on commit 5a377ad

Please sign in to comment.