You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by gallenvara <gi...@git.apache.org> on 2017/01/13 04:29:59 UTC
[GitHub] flink pull request #3110: [FLINK-2184] Cannot get last element with maxBy/mi...
GitHub user gallenvara opened a pull request:
https://github.com/apache/flink/pull/3110
[FLINK-2184] Cannot get last element with maxBy/minBy.
Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration.
If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the [How To Contribute guide](http://flink.apache.org/how-to-contribute.html).
In addition to going through the list, please provide a meaningful description of your changes.
- [X] General
- The pull request references the related JIRA issue ("[FLINK-XXX] Jira title text")
- The pull request addresses only one issue
- Each commit in the PR has a meaningful commit message (including the JIRA id)
- [X] Documentation
- Documentation has been added for new functionality
- Old documentation affected by the pull request has been updated
- JavaDoc for public methods has been added
- [ ] Tests & Build
- Functionality added by the pull request is covered by tests
- `mvn clean verify` has been executed successfully locally or a Travis build has passed
Extend the minBy/maxBy with first parameter. @fhueske sorry for the late update for this pr, can you help with review work?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gallenvara/flink flink-2184
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/3110.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #3110
----
commit 8ae9bb689320a0b5b3fe199ecc61c62504ff0e7d
Author: gallenvara <ga...@126.com>
Date: 2016-05-09T11:51:24Z
Extend minBy/maxBy methods to support returning last element.
commit e04ae6e2bfad1c52460dc742be41153dd012b291
Author: gaolun.gl <ga...@alibaba-inc.com>
Date: 2017-01-13T04:10:08Z
update pr of [FLINK-2184] Cannot get last element with maxBy/minBy.
commit 8bff4dda475bfecaaa2b5efeab84c1afc87d0f5f
Author: gaolun.gl <ga...@alibaba-inc.com>
Date: 2017-01-13T04:12:19Z
Merge remote-tracking branch 'origin/flink-2184' into flink-2184
# Conflicts:
# flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/AllWindowedStream.scala
# flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/KeyedStream.scala
# flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/WindowedStream.scala
commit 71d2c23574d42cedb558ca991ebd87f814a16fde
Author: gaolun.gl <ga...@alibaba-inc.com>
Date: 2017-01-13T04:26:45Z
update pr of [FLINK-2184] Cannot get last element with maxBy/minBy.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3110: [FLINK-2184] Cannot get last element with maxBy/minBy.
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/3110
Thanks @aljoscha, those are my concerns as well. The methods are very prominent in the core APIs and provide only marginal value. It's pretty straightforward to implement the functionality with a `ReduceFunction` in case you need it.
Are there plans to deprecate the corresponding methods on the Java DataStream API?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3110: [FLINK-2184] Cannot get last element with maxBy/minBy.
Posted by gallenvara <gi...@git.apache.org>.
Github user gallenvara commented on the issue:
https://github.com/apache/flink/pull/3110
@fhueske @twalthr . Can you help with reviewing this PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3110: [FLINK-2184] Cannot get last element with maxBy/minBy.
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/3110
Thanks @gallenvara, the changes look good to me but we would also need a few tests.
@aljoscha, what do you think about this change? Should we add it to be consistent with the Java API or do you think it would bloat the API too much? IIRC, there have been some discussions about the built-in aggregation functions.
Best, Fabian
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3110: [FLINK-2184] Cannot get last element with maxBy/minBy.
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/3110
There were, but only in my head. Maybe we should open an issue or discuss on the ML.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3110: [FLINK-2184] Cannot get last element with maxBy/minBy.
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/3110
Thanks for notifying @fhueske! And sorry for not noticing this earlier and jumping on so late.
I think this part of the API is already to bloated and (at the same time) not powerful enough. These functions don't allow having multiple "aggregations" in one operation, i.e. you cannot do `inputStream.keyBy(...).min(1).andMax(2).andSum(3)` (not actual API). I think in the long run we should remove this part of the API because the Table API is a nicer way of expressing simple aggregations. What do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---