Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

executor: remove Next function for HashAggExec #5987

Merged
merged 14 commits into from
Mar 22, 2018
60 changes: 1 addition & 59 deletions executor/aggregate.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,37 +143,7 @@ func (e *HashAggExec) execute(ctx context.Context) (err error) {

// Next implements the Executor Next interface.
func (e *HashAggExec) Next(ctx context.Context) (Row, error) {
Copy link
Member

@zz-jason zz-jason Mar 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can remore Next function totally now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

// In this stage we consider all data from src as a single group.
if !e.executed {
for {
hasMore, err := e.innerNext(ctx)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we remove innerNext ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, let me remove it.

if err != nil {
return nil, errors.Trace(err)
}
if !hasMore {
break
}
}
if (e.groupMap.Len() == 0) && len(e.GroupByItems) == 0 {
// If no groupby and no data, we should add an empty group.
// For example:
// "select count(c) from t;" should return one row [0]
// "select count(c) from t group by c1;" should return empty result set.
e.groupMap.Put([]byte{}, []byte{})
}
e.executed = true
}

groupKey, _ := e.groupIterator.Next()
if groupKey == nil {
return nil, nil
}
retRow := make([]types.Datum, 0, len(e.AggFuncs))
aggCtxs := e.getContexts(groupKey)
for i, af := range e.AggFuncs {
retRow = append(retRow, af.GetResult(aggCtxs[i]))
}
return retRow, nil
return nil, nil
}

func (e *HashAggExec) getGroupKey(row types.Row) ([]byte, error) {
Expand All @@ -196,34 +166,6 @@ func (e *HashAggExec) getGroupKey(row types.Row) ([]byte, error) {
return e.groupKey, nil
}

// innerNext fetches a single row from src and update each aggregate function.
// If the first return value is false, it means there is no more data from src.
func (e *HashAggExec) innerNext(ctx context.Context) (ret bool, err error) {
srcRow, err := e.children[0].Next(ctx)
if err != nil {
return false, errors.Trace(err)
}
if srcRow == nil {
return false, nil
}
e.executed = true
groupKey, err := e.getGroupKey(srcRow)
if err != nil {
return false, errors.Trace(err)
}
if len(e.groupMap.Get(groupKey, e.groupVals[:0])) == 0 {
e.groupMap.Put(groupKey, []byte{})
}
aggCtxs := e.getContexts(groupKey)
for i, af := range e.AggFuncs {
err = af.Update(aggCtxs[i], e.sc, srcRow)
if err != nil {
return false, errors.Trace(err)
}
}
return true, nil
}

func (e *HashAggExec) getContexts(groupKey []byte) []*aggregation.AggEvaluateContext {
groupKeyString := string(groupKey)
aggCtxs, ok := e.aggCtxsMap[groupKeyString]
Expand Down