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

plan: new plan support hash aggregation. #3093

Merged
merged 4 commits into from
Apr 25, 2017
Merged

plan: new plan support hash aggregation. #3093

merged 4 commits into from
Apr 25, 2017

Conversation

hanfei1991
Copy link
Member

Now we only support hash aggregation.
@shenli @coocood @zimulala @tiancaiamao PTAL

@shenli shenli added the priority/P1 The issue has P1 priority. label Apr 24, 2017
if task != nil {
return task, nil
}
task, err = p.convert2HashAggregation(prop)
Copy link
Member

Choose a reason for hiding this comment

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

We will support streaming aggregation latter?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah

}
partialAgg = p.Copy().(*PhysicalAggregation)
// TODO: It's toooooo ugly here. Refactor in the future !!
gkType := types.NewFieldType(mysql.TypeBlob)
Copy link
Member

Choose a reason for hiding this comment

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

This is ported from old code?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah

@shenli
Copy link
Member

shenli commented Apr 25, 2017

LGTM
@zimulala @winoros @coocood PTAL

@hanfei1991 hanfei1991 added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 25, 2017
gkType.Collate = charset.CollationBin
partialSchema := expression.NewSchema(&expression.Column{RetType: gkType, FromID: p.id, Index: 0})
cursor := 0
newAggFuncs := make([]expression.AggregationFunction, len(finalAgg.AggFuncs))
Copy link
Member

Choose a reason for hiding this comment

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

finalAggFunc is a more consistent name.

@coocood
Copy link
Member

coocood commented Apr 25, 2017

rest LGTM

Copy link
Member

@coocood coocood left a comment

Choose a reason for hiding this comment

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

LGTM

@coocood coocood merged commit 1f75f38 into master Apr 25, 2017
@coocood coocood deleted the hanfei/agg branch April 25, 2017 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/P1 The issue has P1 priority. status/LGT1 Indicates that a PR has LGTM 1.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants