Skip to content

[BEAM-2477] BeamAggregationRel should use Combine.perKey instead of GroupByKey#3398

Closed
JingsongLi wants to merge 1 commit intoapache:DSL_SQLfrom
JingsongLi:BEAM-2477
Closed

[BEAM-2477] BeamAggregationRel should use Combine.perKey instead of GroupByKey#3398
JingsongLi wants to merge 1 commit intoapache:DSL_SQLfrom
JingsongLi:BEAM-2477

Conversation

@JingsongLi
Copy link
Contributor

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

  • Make sure the PR title is formatted like:
    [BEAM-<Jira issue #>] Description of pull request
  • Make sure tests pass via mvn clean verify.
  • Replace <Jira issue #> in the title with the actual Jira issue
    number, if there is one.
  • If this contribution is large, please file an Apache
    Individual Contributor License Agreement.

@JingsongLi
Copy link
Contributor Author

R: @xumingmin @xumingming @takidau

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 2ed6075 on JingsongLi:BEAM-2477 into ** on apache:DSL_SQL**.

@JingsongLi
Copy link
Contributor Author

retest this please

1 similar comment
@JingsongLi
Copy link
Contributor Author

retest this please

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 2ed6075 on JingsongLi:BEAM-2477 into ** on apache:DSL_SQL**.

Copy link

@mingmxu mingmxu left a comment

Choose a reason for hiding this comment

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

Thanks @JingsongLi

LGTM, given that Combine.perKey equals with GroupByKey, and there's lot of optimization in Combine.perKey.

@takidau
Copy link
Contributor

takidau commented Jun 22, 2017

Thanks a lot, nice improvement. LGTM. Will merge as soon as I can.

asfgit pushed a commit that referenced this pull request Jun 22, 2017
@takidau
Copy link
Contributor

takidau commented Jun 22, 2017

Merged, feel free to close. Thank you!

@JingsongLi JingsongLi closed this Jun 24, 2017
@JingsongLi JingsongLi deleted the BEAM-2477 branch June 24, 2017 04:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants