-
Notifications
You must be signed in to change notification settings - Fork 1k
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: support WINDOWEND in WHERE of pull queries #5680
feat: support WINDOWEND in WHERE of pull queries #5680
Conversation
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.
LGTM!
.../java/io/confluent/ksql/execution/streams/materialization/ks/KsMaterializedSessionTable.java
Outdated
Show resolved
Hide resolved
@@ -61,28 +63,35 @@ | |||
private List<WindowedRow> findSession( | |||
final ReadOnlySessionStore<Struct, GenericRow> store, | |||
final Struct key, | |||
final Range<Instant> windowStart | |||
final Range<Instant> windowStart, | |||
final Range<Instant> windowEnd | |||
) { | |||
try (KeyValueIterator<Windowed<Struct>, GenericRow> it = store.fetch(key)) { |
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.
Just for my own knowledge, why don't we fetch from the store using the same bounds as in KsMaterializedWindowTable.java
. Is it because the window sizes are not uniform in size and therefore we don't know exactly where to stop if all we know is the window start range? If they provide a window end range now, it seems like we could provide bounds to the store lookup (and could use Long.MAX_VALUE anyway).
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.
The Streams API for session tables is different. There is no fetch(key, lower, upper)
method.
I'm assuming you were meaning to approve this @AlanConfluent and merging. Any other issues just let me know. |
Description
fixes: #5666
Pull queries can now reference WINDOWEND in their WHERE clause, (assuming the source is windowed):
Testing done
usual
Reviewer checklist