You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by ChengXiangLi <gi...@git.apache.org> on 2015/11/18 14:35:17 UTC
[GitHub] flink pull request: [FLINK-2115] [Table API] support no aggregatio...
GitHub user ChengXiangLi opened a pull request:
https://github.com/apache/flink/pull/1377
[FLINK-2115] [Table API] support no aggregation after groupBy.
`JavaBatchTranslator` does not support no aggregation `select` after `groupBy` previously, add the logic in this PR.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ChengXiangLi/flink flink-2115
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/1377.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 #1377
----
commit 461f60ceb5e07ca192451c07813a405b83470acc
Author: chengxiang li <ch...@intel.com>
Date: 2015-11-18T11:02:11Z
[FLINK-2115] [Table API] support no aggregation after groupBy.
----
---
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 pull request: [FLINK-2115] [Table API] support no aggregatio...
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the pull request:
https://github.com/apache/flink/pull/1377#issuecomment-157997049
OK, I see your point. We can go with the MySQL way then. Thanks for explaining it. :smile:
Could you please update the documentation with a small paragraph that explains the behavior?
---
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 pull request: [FLINK-2115] [Table API] support no aggregatio...
Posted by ChengXiangLi <gi...@git.apache.org>.
Github user ChengXiangLi commented on the pull request:
https://github.com/apache/flink/pull/1377#issuecomment-158007073
Ok, i think it could be added in the `Select` operator description in Table API page, it would depends on FLINK-2955. Seems it takes quite long time delay to sync merged commit to github, waiting for FLINK-2955 get synced.
---
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 pull request: [FLINK-2115] [Table API] support no aggregatio...
Posted by ChengXiangLi <gi...@git.apache.org>.
Github user ChengXiangLi closed the pull request at:
https://github.com/apache/flink/pull/1377
---
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 pull request: [FLINK-2115] [Table API] support no aggregatio...
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the pull request:
https://github.com/apache/flink/pull/1377#issuecomment-161265693
Thanks for your efforts. :smile: Could you please close 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 pull request: [FLINK-2115] [Table API] support no aggregatio...
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the pull request:
https://github.com/apache/flink/pull/1377#issuecomment-161235699
Hi, you are are absolutely right, I'm sorry. Please don't hesitate to ping me in the future if I'm taking to long but I'll also try to be more responsive on these.
I'm merging it now. :smile:
---
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 pull request: [FLINK-2115] [Table API] support no aggregatio...
Posted by ChengXiangLi <gi...@git.apache.org>.
Github user ChengXiangLi commented on the pull request:
https://github.com/apache/flink/pull/1377#issuecomment-161190371
Hi, @aljoscha , i guess you may forget this PR :smile: , could you help to move this forward? it should be ready yet.
---
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 pull request: [FLINK-2115] [Table API] support no aggregatio...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1377#issuecomment-158025348
@Li Chengxiang, you can always pull changes directly from the ASF git
repository: https://git-wip-us.apache.org/repos/asf/flink.git. Then you
don't have to wait for syncing with the github mirror.
On Thu, Nov 19, 2015 at 10:51 AM, Li Chengxiang <no...@github.com>
wrote:
> Ok, i think it could be added in the Select operator description in Table
> API page, it would depends on FLINK-2955. Seems it takes quite long time
> delay to sync merged commit to github, waiting for FLINK-2955 get synced.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/flink/pull/1377#issuecomment-158007073>.
>
---
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 pull request: [FLINK-2115] [Table API] support no aggregatio...
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the pull request:
https://github.com/apache/flink/pull/1377#issuecomment-157728542
What it does now is to only pick one element in the group that has the compound key of the `groupBy` expression. The reason why I did not want to support this initially was that the element that results from this has arbitrary values in the fields that are not key fields since only on arbitrary element of the group of elements is picked.
Should we maybe limit this to only allow selecting fields that are also key fields?
---
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 pull request: [FLINK-2115] [Table API] support no aggregatio...
Posted by ChengXiangLi <gi...@git.apache.org>.
Github user ChengXiangLi commented on the pull request:
https://github.com/apache/flink/pull/1377#issuecomment-157932844
@aljoscha , it's a very interesting topic, should we add a limitation that only allow key fields to be no aggregated function field in `select` clause?
Oracle has this limitation while Mysql does not. Most of the time, select non-aggregation field after groupby has no real meaning, while sometimes, if user is aware that groupby some fielde would lead to other fields get grouped as well, this feature would help to improve the performance and user convenience, more described here: http://dev.mysql.com/doc/refman/5.0/en/group-by-handling.html . Currently we keep consistent with Mysql on this feature, allow no aggregation field after `groupBy`, with no guarantee of its return value.
Actually, i prefer to the Mysql way, more flexible to user, 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.
---