aiguofer commented on code in PR #37088:
URL: https://github.com/apache/arrow/pull/37088#discussion_r1290470152
##########
java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrowUtils.java:
##########
@@ -188,12 +188,9 @@ public static ArrowType getArrowTypeFromJdbcType(final
JdbcFieldInfo fieldInfo,
case Types.TIME:
return new ArrowType.Time(TimeUnit.MILLISECOND, 32);
case Types.TIMESTAMP:
- final String timezone;
- if (calendar != null) {
- timezone = calendar.getTimeZone().getID();
- } else {
- timezone = null;
- }
+ return new ArrowType.Timestamp(TimeUnit.MILLISECOND);
+ case Types.TIMESTAMP_WITH_TIMEZONE:
+ final String timezone = calendar == null ? null :
calendar.getTimeZone().getID();
Review Comment:
Hmmm maybe we can change the consumer to do something smarter than
`getTimestamp`. [This SO](https://stackoverflow.com/a/73092871/1815486)
suggests `OffsetDateTime odt = myResultSet.getObject( … , OffsetDateTime.class
) ; `.
So for TZ aware timestamps we could use ^ and if no offset is available
assume UTC. I think that would work for most things, but it'd get tricky for
something like `TIMESTAMP_LTZ`. I wonder how that even comes across through the
JDBC driver? I imagine it's still of type `TIMESTAMP_WITH_TIMEZONE` just like
`TIMESTAMP_TZ`. We might want to add the value from `rsmd.getColumnTypeName` to
the `JdbcFieldInfo` as well so users can disambiguate between the two if they
need to write a custom converter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
