[FLINK-10676][table] Add 'as' method for OverWindowWithOrderBy #6949
Conversation
There was a problem hiding this comment.
Hi, @hequn8128 Thanks for the PR.
The PR LGTM, except for a few comments I left.
Best,
Jincheng
| @@ -18,7 +18,8 @@ | |||
|
|
|||
| package org.apache.flink.table.api.java | |||
|
|
|||
| import org.apache.flink.table.api.{TumbleWithSize, OverWindowWithPreceding, SlideWithSize, SessionWithGap} | |||
| import org.apache.flink.table.api.scala.{CURRENT_RANGE, UNBOUNDED_RANGE} | |||
| import org.apache.flink.table.api._ | |||
There was a problem hiding this comment.
I think using import org.apache.flink.table.api.{OverWindow, TumbleWithSize, OverWindowWithPreceding, SlideWithSize, SessionWithGap} is better than using wildcard, because there are not many classes imported here.What do you think?
docs/dev/table/tableApi.md
Outdated
| <td> | ||
| <p>Defines the interval of rows that are included in the window and precede the current row. The interval can either be specified as time or row-count interval.</p> | ||
|
|
||
| <p><a href="tableApi.html#bounded-over-windows">Bounded over windows</a> are specified with the size of the interval, e.g., <code>10.minutes</code> for a time interval or <code>10.rows</code> for a row-count interval.</p> | ||
|
|
||
| <p><a href="tableApi.html#unbounded-over-windows">Unbounded over windows</a> are specified using a constant, i.e., <code>UNBOUNDED_RANGE</code> for a time interval or <code>UNBOUNDED_ROW</code> for a row-count interval. Unbounded over windows start with the first row of a partition.</p> | ||
|
|
||
| <p>If the <code>preceding</code> and <code>following</code> clause both are omitted, RANGE UNBOUNDED PRECEDING AND CURRENT ROW is used as default for window.</p> |
There was a problem hiding this comment.
UNBOUNDED_RANGE and CURRENT_RANGE appear in pairs，So the following description is recommended:
If the preceding clause is omitted, UNBOUNDED_RANGE and CURRENT_RANGE is used as default for window. What do you think?
| @@ -18,8 +18,8 @@ | |||
|
|
|||
| package org.apache.flink.table.api.scala | |||
|
|
|||
| import org.apache.flink.table.api.{TumbleWithSize, OverWindowWithPreceding, SlideWithSize, SessionWithGap} | |||
| import org.apache.flink.table.expressions.Expression | |||
| import org.apache.flink.table.api._ | |||
There was a problem hiding this comment.
Same as java/windows.scala comments.
|
Hi @sunjincheng121 , thanks a lot for your view. I think all of the suggestions are good! I have addressed the comments and updated the pr. |
|
@hequn8128 Thanks for the updated！ Thanks, |
|
CI find |
|
After the rerun, the error disappeared. no specific reasons are found, so created a JIRA. FLINK-10819， and will continue to pay attention. Merging this PR... |
|
Thanks a lot for the review and merging! |
…uired to optional. This closes apache#6949
What is the purpose of the change
The preceding clause of OVER Window in the traditional database is optional. The default is UNBOUNDED. So we can add the
asmethod toOverWindowWithOrderBy. This way OVER Window is written more easily. e.g.:.window(Over partitionBy 'c orderBy 'proctime preceding UNBOUNDED_ROW as 'w)Can be simplified as follows:
.window(Over partitionBy 'c orderBy 'proctime as 'w)SQL has already supported such feature.
Brief change log
asfunction forOverWindowWithOrderByboth for scala and java.Verifying this change
This change added tests and can be verified as follows:
OverWindowTestandOverWindowStringExpressionTest.Does this pull request potentially affect one of the following parts:
@Public(Evolving): (yes)Documentation