-
Notifications
You must be signed in to change notification settings - Fork 4k
GH-33143: [C++] Kernel to convert timestamp with timezone to wall time #34208
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -672,6 +672,23 @@ struct IsDaylightSavings { | |
| const time_zone* tz_; | ||
| }; | ||
|
|
||
| // ---------------------------------------------------------------------- | ||
| // Extract local time of a given timestamp given its timezone | ||
|
|
||
| template <typename Duration, typename Localizer> | ||
| struct LocalTime { | ||
| explicit LocalTime(const FunctionOptions* options, Localizer&& localizer) | ||
| : localizer_(std::move(localizer)) {} | ||
|
|
||
| template <typename T, typename Arg0> | ||
| T Call(KernelContext*, Arg0 arg, Status*) const { | ||
| const auto t = localizer_.template ConvertTimePoint<Duration>(arg); | ||
| return static_cast<T>(t.time_since_epoch().count()); | ||
| } | ||
|
|
||
| Localizer localizer_; | ||
| }; | ||
|
|
||
| // ---------------------------------------------------------------------- | ||
| // Round temporal values to given frequency | ||
|
|
||
|
|
@@ -1327,6 +1344,12 @@ Result<TypeHolder> ResolveAssumeTimezoneOutput(KernelContext* ctx, | |
| return timestamp(in_type.unit(), AssumeTimezoneState::Get(ctx).timezone); | ||
| } | ||
|
|
||
| Result<TypeHolder> ResolveLocalTimeOutput(KernelContext* ctx, | ||
| const std::vector<TypeHolder>& args) { | ||
| const auto& in_type = checked_cast<const TimestampType&>(*args[0]); | ||
| return timestamp(in_type.unit()); | ||
| } | ||
|
|
||
| template <typename Duration> | ||
| struct AssumeTimezone { | ||
| explicit AssumeTimezone(const AssumeTimezoneOptions* options, const time_zone* tz) | ||
|
|
@@ -1784,6 +1807,13 @@ const FunctionDoc is_dst_doc{ | |
| "An error is returned if the values do not have a defined timezone."), | ||
| {"values"}}; | ||
|
|
||
| const FunctionDoc local_time_doc{ | ||
| "Convert timestamp to a timezone-naive local time timestamp", | ||
| ("LocalTime converts a timestamp to a local time of timestamps timezone\n" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something is missing in this sentence? |
||
| "and removes timezone metadata. If input is in UTC or doesn't have\n" | ||
| "timezone metadata, it is returned as is.\n" | ||
|
Comment on lines
+1813
to
+1814
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Returned "as is" is not entirely correct for UTC, as I assume the "UTC" timezone is still removed? |
||
| "Null values emit null."), | ||
| {"values"}}; | ||
| const FunctionDoc floor_temporal_doc{ | ||
| "Round temporal values down to nearest multiple of specified time unit", | ||
| ("Null values emit null.\n" | ||
|
|
@@ -1801,8 +1831,9 @@ const FunctionDoc ceil_temporal_doc{ | |
| const FunctionDoc round_temporal_doc{ | ||
| "Round temporal values to the nearest multiple of specified time unit", | ||
| ("Null values emit null.\n" | ||
| "An error is returned if the values have a defined timezone but it\n" | ||
| "cannot be found in the timezone database."), | ||
| "If timezone is not given then timezone naive timestamp in UTC are\n" | ||
| "returned. An error is returned if the values have a defined timezone\n" | ||
| "but it cannot be found in the timezone database."), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was this change intentional? Or was it meant to be in |
||
| {"timestamps"}, | ||
| "RoundTemporalOptions"}; | ||
|
|
||
|
|
@@ -1969,6 +2000,12 @@ void RegisterScalarTemporalUnary(FunctionRegistry* registry) { | |
| is_dst_doc); | ||
| DCHECK_OK(registry->AddFunction(std::move(is_dst))); | ||
|
|
||
| auto local_time = | ||
| UnaryTemporalFactory<LocalTime, TemporalComponentExtract, TimestampType>::Make< | ||
| WithTimestamps>("local_time", OutputType::Resolver(ResolveLocalTimeOutput), | ||
| local_time_doc); | ||
| DCHECK_OK(registry->AddFunction(std::move(local_time))); | ||
|
|
||
| // Temporal rounding functions | ||
| // Note: UnaryTemporalFactory will not correctly resolve OutputType(FirstType) to | ||
| // output type. See TemporalComponentExtractRound for more. | ||
|
|
||
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.
I was going to suggest removing this but now I see this is everywhere. Hmm.... :( I'm not sure how true this is these days.
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.
Well, you could see it as an opportunity to sync the API with substrait :D