Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-6011] Support TUMBLE, HOP, SESSION window in streaming SQL. #3665

Closed
wants to merge 2 commits into from

Conversation

@haohui
Copy link

haohui commented Apr 3, 2017

This PR adds supports for the TUMBLE, HOP, and SESSION windows in Flink.

The work of supporting WindowStart and WindowEnd expressions will be deferred to FLINK-6012.

@fhueske
Copy link
Contributor

fhueske commented Apr 4, 2017

Thanks for the PR @haohui!
+1 to merge.
I've been waiting for this feature :-)

@shaoxuan-wang
Copy link
Contributor

shaoxuan-wang commented Apr 4, 2017

So we finally got those supported by Calcite 1.12?Really excited to see those features supported in flinkSQL. Thanks @haohui.

Copy link
Contributor

twalthr left a comment

Thanks @haohui. Great that we support windows in SQL soon. I had some minor comments. Could you also add some documentation in the SQL section (maybe by copying paragraphs from the Table API section)?

ProcTimeExtractor
ProcTimeExtractor,
SqlStdOperatorTable.TUMBLE,
SqlStdOperatorTable.TUMBLE_START,

This comment has been minimized.

Copy link
@twalthr

twalthr Apr 4, 2017

Contributor

Do we already support the START/END functions? We should let them unsupported until they are implemented and tested.


private[flink] val INSTANCE = new LogicalWindowAggregateRule
protected def getOperandAsLong(idx: Int): Long =
unwrapLiteral[BigDecimal](call.getOperands.get(idx)).longValue()

This comment has been minimized.

Copy link
@twalthr

twalthr Apr 4, 2017

Contributor

Does Calcite ensure that operands can only be literals, no input reference?

This comment has been minimized.

Copy link
@haohui

haohui Apr 5, 2017

Author

I just tried out Calcite did not stop you from passing something like a {{RexCall}}. So yes, it can be dynamic.

One question: whether Flink actually supports GroupWindow that has a dynamic size? Maybe I'm wrong but it does not seem so.

If the answer is no maybe we should check that whether it is a RexLiteral?

This comment has been minimized.

Copy link
@fhueske

fhueske Apr 5, 2017

Contributor

Flink does only support windows with fixed configuration (SESSION windows have variable length, but the gap parameter is fixed). I'm also not sure if that would make sense. It's quite hard to reason about the behavior of a window with variable parameters, IMO.

This comment has been minimized.

Copy link
@haohui

haohui Apr 5, 2017

Author

Agree. 97d1a45 will throw an TableException if the configuration is not fixed.

Haohui Mai
@fhueske
Copy link
Contributor

fhueske commented Apr 5, 2017

Thanks for the update @haohui!
Will merge this PR.

fhueske added a commit to fhueske/flink that referenced this pull request Apr 5, 2017
…ns in SQL queries on streams.

This closes apache#3665.
@asfgit asfgit closed this in 1649f35 Apr 5, 2017
heytitle added a commit to heytitle/flink that referenced this pull request Apr 16, 2017
hequn8128 pushed a commit to hequn8128/flink that referenced this pull request Jun 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.