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-23582][docs][table] Add documentation for session window tvf. #16963
Conversation
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit fd63386 (Tue Aug 24 10:52:32 UTC 2021) ✅no warnings Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
fd63386
to
6a8c98e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution, @beyond1920. I left some minor comments
@godfreyhe Thanks for review, I've updated. Please have a look. |
cc @wuchong |
The documentation looks good to me. However, it seems that the session window TVF syntax missed to declare partition key. This introduces a wrong sematic when assiging session windows. We should fix it ASAP. |
-- apply aggregation on the session windowed table | ||
> SELECT window_start, window_end, SUM(price) | ||
FROM TABLE( | ||
SESSION(TABLE Bid, DESCRIPTOR(bidtime), INTERVAL '3' MINUTES)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you update the example to show a partitioned session window? I think that is a more usable use case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
bb11825
to
90ddba0
Compare
90ddba0
to
e5730d5
Compare
@wuchong Jark, I've updated the pr to add an example to show a partitioned session window and rebase, please have a look again. |
e5730d5
to
3f642de
Compare
3f642de
to
555342f
Compare
@wuchong can we merge this PR? |
@tillrohrmann , we discussed to rework the syntax of session window TVF in FLINK-24024. There would be a new docs PR to illustrate the new syntax. So I think we can close this PR for now. What do you think @beyond1920 ? |
@wuchong @tillrohrmann I prefer to close this pull request. I would create a new pull request after we upgrade the syntax of session window. |
What is the purpose of the change
This pr adds documentation for session window tvf.
Brief change log
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (no)Documentation