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.

Sign up for GitHub

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

Jump to bottom

Add non ICU time_bucket function #5835

Merged
merged 36 commits into from
Jan 7, 2023
Merged

Add non ICU time_bucket function #5835

merged 36 commits into from
Jan 7, 2023

Conversation

papparapa
Copy link
Contributor

#5589

Split from PR #5665.

While ICU time_bucket implementation in #5665 still has some problems, non-ICU time_bucket seems ready.
This PR adds only non-ICU time_bucket functions.

They behave almost same as TimescaleDB's time_bucket.

TIME_BUCKET(bucket_width: INTERVAL, ts: TIMESTAMP)
TIME_BUCKET(bucket_width: INTERVAL, ts: DATE)

TIME_BUCKET(bucket_width: INTERVAL, ts: TIMESTAMP, offset: INTERVAL)
TIME_BUCKET(bucket_width: INTERVAL, ts: DATE, offset: INTERVAL)

TIME_BUCKET(bucket_width: INTERVAL, ts: TIMESTAMP, origin: TIMESTAMP)
TIME_BUCKET(bucket_width: INTERVAL, ts: DATE, origin: DATE)

@papparapa
Add time_bucket function for DATE and TIMESTAMP
8c5d032
@papparapa
Add time_bucket function without timezone argument for TIMESTAMPTZ
6cf668a
@papparapa
Fix handling null
31eed85
@papparapa
Move time_bucket for TIMESTANP_TZ to ICU extension
da7f0b4
@papparapa
Add DUCKDB_API specifier to ConvertToString::Operation specializations
981f802
@papparapa
Avoid throwing error when argument is NULL in time_bucket for instants
6d0381f
@papparapa
Move ICUCalendarAdd and ICUCalendarSub to header
5d16534
@papparapa
Remove unnecessary DUCKDB_API specifiers in interval.hpp
5f5ab3d
@papparapa
Refactor time_bucket.cpp
7b8f921
@papparapa
Change bucket algorithm in icu-timebucket.cpp
d2ffa89
@papparapa
Fix placement of ICUCalendarAdd and ICUCalendarSub
79037f8
@papparapa
Refactor time_bucket.cpp
aad4028
@papparapa
Fix test cases for instants
7ce10f3
@papparapa
Fix icu-tumebucket.cpp to compile
9f9412b
@papparapa
Merge branch 'master' of https://github.com/duckdb/duckdb into add-ti…
3ff3cc6 
…me-bucket-function
@papparapa
Fix expected timestamps format
83a5252
@papparapa
Fix time_bucket.cpp for tidy check
5adc89e
@papparapa
Fix icu-timebucket.cpp to compile
2585a1d
@papparapa
Fix bucket algorithm in icu-timebucket.cpp
09c0157
@papparapa
Wrap ICUCalendarAdd/Sub::Operation to ICUDateFunc::Add/Sub
b261414
@papparapa
Fix format
8f3b85e
@papparapa
Remove unnecessary templates
a4139cd
@papparapa
Add wrapper of ICUFromLocalTimestamp::Operation to ICUDateFunc
9ac9df8
@papparapa
Add origin timezone offset
83245de
@papparapa
Fix test cases for TIMESTAMPTZ
1a5c03f
@papparapa
Add test result column helpful for debug
84e17a9
@papparapa
Rewrite bucket algorithm to avoid bugs related to daylight saving time
6c83b13
@papparapa
Merge branch 'master' of https://github.com/duckdb/duckdb into add-ti…
f0847df 
…me-bucket-function
@papparapa
Remove unnecessary overloads
f57b31b
@papparapa
Add daylight saving time boundary test
7b4bf2a
@hawkfish hawkfish self-requested a review January 6, 2023 20:54
hawkfish
hawkfish reviewed Jan 6, 2023
View reviewed changes
Copy link
Contributor

@hawkfish hawkfish left a comment

Choose a reason for hiding this comment

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

Looks pretty good - just use the library functions that check overflow.

src/function/scalar/date/time_bucket.cpp Outdated Show resolved Hide resolved
src/function/scalar/date/time_bucket.cpp Outdated Show resolved Hide resolved
src/function/scalar/date/time_bucket.cpp Outdated Show resolved Hide resolved
@Mytherin Mytherin merged commit 6be5ebf into duckdb:master Jan 7, 2023
@Mytherin
Copy link
Collaborator

Mytherin commented Jan 7, 2023

Thanks!

@papparapa papparapa deleted the add-non-icu-time-bucket-function branch January 8, 2023 00:22
@papparapa papparapa mentioned this pull request Jan 8, 2023
Merged
@papparapa papparapa mentioned this pull request Jan 22, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hawkfish hawkfish hawkfish approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@papparapa @Mytherin @hawkfish