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

Windowed aggregations: calculate daily aggregates based on time zones other than UTC #1968

Open
rmoff opened this issue Oct 1, 2018 · 8 comments

Comments

@rmoff
Copy link
Contributor

@rmoff rmoff commented Oct 1, 2018

Per StackOverflow question, if calculating aggregates with KSQL at day-level and above (day/week/month/etc) it would be useful to be able to do these based on the local timezone.

KSQL windows are based on the epoch (number of milliseconds since Jan 01 1970, UTC).

For aggregates at a second/minute/hour level this doesn't really matter, but for an organisation wanting to report on "yesterday" etc then the window would need to include events within the 24 hour time window based on the specified timezone, not necessarily UTC.

One idea is if KSQL supported a DATEADD function, where the event timestamp could be shifted by the required hours to match the local timezone relative to UTC, and then that used as the timestamp for aggregations. However, this feels like a bit of a hack—open to better suggestions for doing this properly.

@miguno

This comment has been minimized.

Copy link
Member

@miguno miguno commented Oct 23, 2018

+1 from a user in an offline conversation at a meetup in Munich.

Feedback was: "Have the ability to specify either the desired time zone or a time offset for windowed aggregations"

@miguno miguno changed the title Calculate daily aggregates based on local timezone Windowed aggregations: calculate daily aggregates based on local timezone Oct 23, 2018
@miguno miguno changed the title Windowed aggregations: calculate daily aggregates based on local timezone Windowed aggregations: calculate daily aggregates based on timezones other than UTC Nov 23, 2018
@miguno miguno changed the title Windowed aggregations: calculate daily aggregates based on timezones other than UTC Windowed aggregations: calculate daily aggregates based on time zones other than UTC Nov 23, 2018
@miguno

This comment has been minimized.

Copy link
Member

@miguno miguno commented Nov 26, 2018

+1 from an offline user conversation, where the goal is to do windowed aggregations where windows start at midnight Eastern time.

@mjsax

This comment has been minimized.

Copy link
Member

@mjsax mjsax commented Nov 26, 2018

This should be addressed in Kafka Streams first IMHO -- the feature request exist there, too. Afterwards, KSQL can expose it to the users.

@miguno

This comment has been minimized.

Copy link
Member

@miguno miguno commented Nov 26, 2018

Agreed @mjsax

@vcrfxia

This comment has been minimized.

Copy link
Contributor

@vcrfxia vcrfxia commented Nov 1, 2019

Looks like there's a workaround in Kafka Streams using custom TimestampExtractors so it is an option to support this in KSQL, though it'd be somewhat hacky.

@mjsax

This comment has been minimized.

Copy link
Member

@mjsax mjsax commented Nov 1, 2019

That's super hacky and I would not recommend it. Why not just work on https://issues.apache.org/jira/browse/KAFKA-7911 directly? Also note, that there is https://github.com/confluentinc/kafka-streams-examples/blob/5.3.1-post/src/test/java/io/confluent/examples/streams/window/DailyTimeWindows.java that we could leverage.

@vcrfxia

This comment has been minimized.

Copy link
Contributor

@vcrfxia vcrfxia commented Nov 1, 2019

Thanks for the update @mjsax . I agree those are both much better options.

@rmoff

This comment has been minimized.

Copy link
Contributor Author

@rmoff rmoff commented Jan 9, 2020

We should also support MONTH and YEAR for window sizes - it sounds like these would fall into this same bucket of work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.