-
Notifications
You must be signed in to change notification settings - Fork 71
Closed
Labels
Type: enhancementNew feature or requestNew feature or request
Description
Describe the enhancement requested
Currently, our Timezone handling is pretty iffy as identified in a few different threads. I've been doing some digging and would like to propose a few changes. I will make a draft PR with how I think we should implement these changes so we can open up discussions on each point.
Support java.time
objects
The JDBC 4.2 spec specifies users can fetch java.time
objects using resultSet.getObject(..., <java.time>.class)
. We should support OffsetDateTime
, ZonedDateTime
, Instant
, and LocalDateTime
for timestamps. We should also support LocalDate
for dates vectors, and LocalTime
for times.
Support TIMESTAMP_WITH_TIMEZONE
JDBC type
Timestamp vectors that include a timezone should be treated as TIMESTAMP_WITH_TIMEZONE
- When retrieved as
OffsetDateTime
orZonedDateTime
, use the TZ included in the vector - When retrieved as
Timestamp
with a Calendar, adjust the value to display time in the TZ specified by the Calendar, assuming the value in the vector is in the vector specified TZ - When retrieved as
LocalDateTime
, do same asTimestamp
but assume the Calendar specifies UTC. - Still not sure how
Instant
plays into all of this
Timestamp vectors without a timezone should be treated as TIMESTAMP
- When retrieved as
OffsetDateTime
orZonedDateTime
, throw an error? - When retrieved as
Timestamp
orLocalDateTime
, take the value as-is and don't do any TZ conversion. The assumption should be that if the vector didn't include TZ, we're just dealing with "wall-clock" time so it should never be adjusted - Still not sure how
Instant
plays into all of this
If we get a new vector type that includes TZ/Offset per record (apache/arrow#44248), it should also be treated as TIMESTAMP_WITH_TIMEZONE
- This is still a hypothetical, but I think if this comes to fruition we should treat it similar to Timestamp vectors with TZ, but use the TZ per record instead of for the whole vector.
Don't do any TZ adjustments for regular TIMESTAMP
w/o TZ
- We currently assume UTC for vectors w/o a TZ and still do an offset conversion when requested with a given Calendar. We should not do any TZ conversion for these since they should be treated as "wall-clock" time.
Parameter binding
We probably also need to deal with relevant changes when binding parameters, but I'll leave that as a separate change since we also need to deal with #153.
Did I miss anything? Let me know!
Metadata
Metadata
Assignees
Labels
Type: enhancementNew feature or requestNew feature or request