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

feat: Add window size for time window tables #3102

Conversation

hjafarpour
Copy link
Contributor

Description

Currently, when we specify WINDOW_TYPE in a DDL statement to declare a table over a topic with WINDOWED key we need to provide the WINDOW_TYPE config property in the WITH clause. However, if the window is a time window, we user should also provide the size of the window, otherwise, the size will be the default, which is Long.MAX_VALUE which is incorrect.
This PR introduces a new config property for DDL statements, WINDOW_SIZE where if the window type is time window, user should provide the size using this parameter in the WITH clause of the DDL statement.

Note that since the current behavior is incorrect anyway, we don't support backward compatibility here.

Also before we support this functionality we either have to have KIP-300 in kafka streams resolved or have a work around in KSQL for it.
Merging this PR won't have adverse effect on the current state anyway.

Testing done

Unit tests were added. QTT tests were updated accordingly.

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).

@hjafarpour hjafarpour requested a review from a team July 19, 2019 19:42
@hjafarpour hjafarpour requested a review from JimGalasyn as a code owner July 19, 2019 19:42
@hjafarpour hjafarpour added this to the 5.4 milestone Jul 19, 2019
Copy link
Contributor

@big-andy-coates big-andy-coates left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @hjafarpour LGTM to me except from some code duplication.

@big-andy-coates big-andy-coates requested a review from a team July 19, 2019 20:23
Copy link
Member

@JimGalasyn JimGalasyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hjafarpour hjafarpour merged commit 6ff07d5 into confluentinc:master Jul 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants