You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 or ZonedDateTime, 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 as Timestamp 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 or ZonedDateTime, throw an error?
When retrieved as Timestamp or LocalDateTime, 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!
The text was updated successfully, but these errors were encountered:
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
objectsThe JDBC 4.2 spec specifies users can fetch
java.time
objects usingresultSet.getObject(..., <java.time>.class)
. We should supportOffsetDateTime
,ZonedDateTime
,Instant
, andLocalDateTime
for timestamps. We should also supportLocalDate
for dates vectors, andLocalTime
for times.Support
TIMESTAMP_WITH_TIMEZONE
JDBC typeTimestamp vectors that include a timezone should be treated as
TIMESTAMP_WITH_TIMEZONE
OffsetDateTime
orZonedDateTime
, use the TZ included in the vectorTimestamp
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 TZLocalDateTime
, do same asTimestamp
but assume the Calendar specifies UTC.Instant
plays into all of thisTimestamp vectors without a timezone should be treated as
TIMESTAMP
OffsetDateTime
orZonedDateTime
, throw an error?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 adjustedInstant
plays into all of thisIf we get a new vector type that includes TZ/Offset per record (apache/arrow#44248), it should also be treated as
TIMESTAMP_WITH_TIMEZONE
Don't do any TZ adjustments for regular
TIMESTAMP
w/o TZParameter 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!
The text was updated successfully, but these errors were encountered: