You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by sunjincheng121 <gi...@git.apache.org> on 2017/05/02 09:47:45 UTC
[GitHub] flink pull request #3806: [FLINK-6426][table]Update the document of group-wi...
GitHub user sunjincheng121 opened a pull request:
https://github.com/apache/flink/pull/3806
[FLINK-6426][table]Update the document of group-window table API&SQL
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.
- [ ] 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
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sunjincheng121/flink FLINK-6426-PR
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/3806.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 #3806
----
commit 7c69eb371731fd3d88c8d8b31269dfde68dcd5bc
Author: sunjincheng121 <su...@gmail.com>
Date: 2017-05-02T09:32:36Z
[FLINK-6426][table]Update the document of group-window table API&SQL
----
---
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 #3806: [FLINK-6426][table]Update the document of group-wi...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/3806#discussion_r115471208
--- Diff: docs/dev/table_api.md ---
@@ -1099,14 +1099,14 @@ Temporal intervals can be represented as number of months (`Types.INTERVAL_MONTH
The Table API is a declarative API to define queries on batch and streaming tables. Projection, selection, and union operations can be applied both on streaming and batch tables without additional semantics. Aggregations on (possibly) infinite streaming tables, however, can only be computed on finite groups of records. Window aggregates group rows into finite groups based on time or row-count intervals and evaluate aggregation functions once per group. For batch tables, windows are a convenient shortcut to group records by time intervals.
-Windows are defined using the `window(w: Window)` clause and require an alias, which is specified using the `as` clause. In order to group a table by a window, the window alias must be referenced in the `groupBy(...)` clause like a regular grouping attribute.
+Windows are defined using the `window(w: Window)` clause and the window must have an alias. In order to group a table by a window, the window alias must be referenced in the `groupBy(...)` clause like a regular grouping attribute.
The following example shows how to define a window aggregation on a table.
<div class="codetabs" markdown="1">
<div data-lang="java" markdown="1">
{% highlight java %}
Table table = input
- .window([Window w].as("w")) // define window with alias w
+ .window([WindowWithoutAlias w].as("w")) // define window with alias w
--- End diff --
I think it is easier to understand to describe the Alias not as part of the window but only as assigning a name to the window. I would like to keep the previous version.
---
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 #3806: [FLINK-6426][table]Update the document of group-window ta...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/3806
Did this PR become obsolete with the recent restructuring of the Table API docs?
Can you check that and close this PR if that's the case @sunjincheng121?
Thank you, 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 pull request #3806: [FLINK-6426][table]Update the document of group-wi...
Posted by sunjincheng121 <gi...@git.apache.org>.
Github user sunjincheng121 commented on a diff in the pull request:
https://github.com/apache/flink/pull/3806#discussion_r116146239
--- Diff: docs/dev/table_api.md ---
@@ -1485,14 +1485,14 @@ Joins, set operations, and non-windowed aggregations are not supported yet.
{% top %}
-### Group Windows
+### Windows
--- End diff --
I'd like called them `Windows`. All of the names are `Tumble`,`Slide`,`Session` and `Over`. It's consistent with the interface of the table API. Because we only have`window(...)` method in `Table.scala`. although the param of `window(overWindows: OverWindow*)` and `window(window: Window)` a little difference.
When user using the tableAPI/SQL they only notice that there are 4 kinds of windows in FLINK.
They are `Tumble`, `Slide/Hup`, `Session` and `Over`. We'd better not reflect the concept of group window. What do you think?
Thanks,
SunJincheng
---
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 #3806: [FLINK-6426][table]Update the document of group-wi...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/3806#discussion_r115470902
--- Diff: docs/dev/table_api.md ---
@@ -1485,14 +1485,14 @@ Joins, set operations, and non-windowed aggregations are not supported yet.
{% top %}
-### Group Windows
+### Windows
--- End diff --
I think we should stick to `Group Windows` here and rather adapt the description of the Table API windows.
In SQL, the term `WINDOW` refers to the `OVER` windows, so I think it is less ambiguous if we call them `Group Windows` and `Over Windows`.
---
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 #3806: [FLINK-6426][table]Update the document of group-wi...
Posted by sunjincheng121 <gi...@git.apache.org>.
Github user sunjincheng121 commented on a diff in the pull request:
https://github.com/apache/flink/pull/3806#discussion_r116146470
--- Diff: docs/dev/table_api.md ---
@@ -1099,14 +1099,14 @@ Temporal intervals can be represented as number of months (`Types.INTERVAL_MONTH
The Table API is a declarative API to define queries on batch and streaming tables. Projection, selection, and union operations can be applied both on streaming and batch tables without additional semantics. Aggregations on (possibly) infinite streaming tables, however, can only be computed on finite groups of records. Window aggregates group rows into finite groups based on time or row-count intervals and evaluate aggregation functions once per group. For batch tables, windows are a convenient shortcut to group records by time intervals.
-Windows are defined using the `window(w: Window)` clause and require an alias, which is specified using the `as` clause. In order to group a table by a window, the window alias must be referenced in the `groupBy(...)` clause like a regular grouping attribute.
+Windows are defined using the `window(w: Window)` clause and the window must have an alias. In order to group a table by a window, the window alias must be referenced in the `groupBy(...)` clause like a regular grouping attribute.
The following example shows how to define a window aggregation on a table.
<div class="codetabs" markdown="1">
<div data-lang="java" markdown="1">
{% highlight java %}
Table table = input
- .window([Window w].as("w")) // define window with alias w
+ .window([WindowWithoutAlias w].as("w")) // define window with alias w
--- End diff --
Yes, But the `as` method does not belong to `Window`. I think the user may has a little bit confused.
---
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 #3806: [FLINK-6426][table]Update the document of group-wi...
Posted by sunjincheng121 <gi...@git.apache.org>.
Github user sunjincheng121 closed the pull request at:
https://github.com/apache/flink/pull/3806
---
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 #3806: [FLINK-6426][table]Update the document of group-window ta...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/3806
Hi @sunjincheng121, thanks for the PR.
I would like to keep the term `Group Window` and think that `WindowWithoutAlias` might be confusing. So, I'd rather keep the docs as they are. We will need to go over this anyways when we add Over windows and non-windowed aggregates.
What do you think?
Thanks, 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.
---