Add support for hour(timestamp_ntz)#1974
Conversation
|
|
||
| FOLLY_ALWAYS_INLINE void call( | ||
| int32_t& result, | ||
| const arg_type<TimestampNTZ>& timestamp) { |
There was a problem hiding this comment.
Sorry for the confusion, we need to use TimestampUtcType. TimestampNTZ is being removed (see facebookincubator#16886). You might could refer to struct TimestampPlusOneFunction for how to register a function for both Timestamp and TimestampUtc types.
There was a problem hiding this comment.
No worries, switched to TimestampUtcType following the TimestampPlusOneFunction pattern
Mariamalmesfer
force-pushed
the
ts-ntz-hour-support
branch
from
484eb10 to
5b830a8
Compare
|
@Mariamalmesfer Thanks for the update. Could you please update the relevant documentation? Also consider opening a corresponding Gluten PR to enable this function. |
Mariamalmesfer
force-pushed
the
ts-ntz-hour-support
branch
6 times, most recently
from
bedb5e9 to
ac505e1
Compare
Thanks! @rui-mo Updated the docs and opened the Gluten PR here: apache/gluten#12064 |
rui-mo
left a comment
rui-mo
left a comment
There was a problem hiding this comment.
Thanks. The corresponding type has already been merged into Meta/Velox, so we can move this PR there now.
| .. spark:function:: hour(timestamp_ntz) -> integer | ||
|
|
||
| Returns the hour of ``timestamp_ntz``. The result is not affected by the | ||
| session timezone since ``timestamp_ntz`` represents wall-clock time with no |
There was a problem hiding this comment.
timestamp_ntzrepresents wall-clock time with no timezone attached.
This seems inaccurate because both timestamp and timestamp_ntz don't carry timezone information. Consider revising it to: timestamp_ntz represents a local time that is independent of time zone.
| @@ -177,6 +177,14 @@ These functions support TIMESTAMP and DATE input types. | |||
|
|
|||
| SELECT hour('2009-07-30 12:58:59'); -- 12 | |||
There was a problem hiding this comment.
Consider adding a timezone aware example for hour(timestamp) to illustrate the difference.
Mariamalmesfer
force-pushed
the
ts-ntz-hour-support
branch
from
ac505e1 to
da15760
Compare
Thanks! Great news. I'll move this PR to facebookincubator/velox Should I also move the casting PR #1992 there as well? |
2 participants
No description provided.