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: support WINDOWEND in WHERE of pull queries #5680

Merged
merged 2 commits into from
Jun 25, 2020

Conversation

big-andy-coates
Copy link
Contributor

Description

fixes: #5666

Pull queries can now reference WINDOWEND in their WHERE clause, (assuming the source is windowed):

SELECT * FROM T
   WHERE id = 10
   AND 134874632 <= WINDOWSTART AND WINDOWEND < 134891111;

Testing done

usual

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • Ensure relevant issues are linked (description should include text like "Fixes #")

@big-andy-coates big-andy-coates requested review from JimGalasyn and a team as code owners June 24, 2020 23:13
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!

@@ -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)) {
Copy link
Member

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).

Copy link
Contributor Author

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.

@big-andy-coates
Copy link
Contributor Author

I'm assuming you were meaning to approve this @AlanConfluent and merging. Any other issues just let me know.

@big-andy-coates big-andy-coates merged commit 40f2f13 into confluentinc:master Jun 25, 2020
@big-andy-coates big-andy-coates deleted the pull_window_end branch June 25, 2020 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pull queries against session window end boundaries
3 participants