asfimport opened a new issue, #324:
URL: https://github.com/apache/arrow-java/issues/324

   It seems that the work which has been tracked in ARROW-2015 and merged in 
<https://github.com/apache/arrow/pull/2966> to change the return types of the 
various Time and Date vector types when using the getObject API missed some of 
the vector types which are temporal and so should return a temporal type, and 
provided an incorrect implementation for others (some of this was pointed out 
in the initial PR review, but it seems that it slipped through the cracks and 
was not addressed before merging).
   
   Here is a table of the various temporal vector types, what they currently 
return from getObject, and what they should return, in my opinion (I have 
included ones in which the implementation is correct for completeness, and 
coloured them green).
   
    
   
    
   
   |Vector class|Current return type|Proposed return type|Comments| |
   |-|-|-|-|-|
   |DateDayVector|Integer|LocalDate|Currently returns the raw value of days 
since epoch, should return the actual date| |
   |DateMilliVector|LocalDateTime|LocalDate|This type is supposed to encode a 
date, not a datetime, so even though epoch millis are used, the result should 
be a LocalDate| |
   |<font color="#00875a">DurationVector</font>|<font 
color="#00875a">Duration</font>|<font color="#00875a">Duration</font>|<font 
color="#00875a">Correct.</font>| |
   |IntervalDayVector|Duration|Period|As per 
<https://github.com/apache/arrow/blob/master/format/Schema.fbs#L251> , Interval 
should be a calendar-based datatype, not a time-based one. This is represented 
in Java by a Period type. However, I note that the Java Period class does not 
support milliseconds, unlike the Arrow type, which might be why Duration is 
being returned. Some discussion may be needed on the best way to deal with 
this.| |
   |<font color="#00875a">IntervalYearVector</font>|<font 
color="#00875a">Period</font>|<font color="#00875a">Period</font>|<font 
color="#00875a">Correct.</font>| |
   |TimeMicroVector|Long|LocalTime|Currently returns the raw number of micros, 
should return the actual time| |
   |TimeMilliVector|LocalDateTime|LocalTime|Currently returns a datetime on 
1970-01-01 with the correct time component, should just return the time| |
   |TimeNanoVector|Long|LocalTime|Currently returns the raw number of nanos, 
should return the actual time| |
   |TimeSecVector|Integer|LocalTime|Currently returns the raw number of 
seconds, should return the actual time| |
   |<font color="#00875a">TimeStampMicroVector</font>|<font 
color="#00875a">LocalDateTime</font>|<font 
color="#00875a">LocalDateTime</font>|<font color="#00875a">Correct.</font>| |
   |<font color="#00875a">TimeStampMilliVector</font>|<font 
color="#00875a">LocalDateTime</font>|<font 
color="#00875a">LocalDateTime</font>|<font color="#00875a">Correct.</font>| |
   |<font color="#00875a">TimeStampNanoVector</font>|<font 
color="#00875a">LocalDateTime</font>|<font 
color="#00875a">LocalDateTime</font>|<font color="#00875a">Correct.</font>| |
   |<font color="#00875a">TimeStampSecVector</font>|<font 
color="#00875a">LocalDateTime</font>|<font 
color="#00875a">LocalDateTime</font>|<font color="#00875a">Correct.</font>| |
   |TimeStampMicroTZVector|Long|ZonedDateTime|Currently returns the underlying 
micros, and TZ has to be obtained separately. Should return the actual datetime 
with timezone| |
   |TimeStampMilliTZVector|Long|ZonedDateTime|Currently returns the underlying 
millis, and TZ has to be obtained separately. Should return the actual datetime 
with timezone| |
   |TimeStampNanoTZVector|Long|ZonedDateTime|Currently returns the underlying 
nanos, and TZ has to be obtained separately. Should return the actual datetime 
with timezone| |
   |TimeStampSecTZVector|Long|ZonedDateTime|Currently returns the underlying 
seconds, and TZ has to be obtained separately. Should return the actual 
datetime with timezone| <br> <br>I am happy to submit a PR to fix this if there 
is no other reason for this to persist other than these types being rarely 
used, but note that these would all be breaking API changes. <br> <br> |
   
   
   **Reporter**: [Matt 
Jadczak](https://issues.apache.org/jira/browse/ARROW-9915)
   
   <sub>**Note**: *This issue was originally created as 
[ARROW-9915](https://issues.apache.org/jira/browse/ARROW-9915). Please see the 
[migration documentation](https://github.com/apache/arrow/issues/14542) for 
further details.*</sub>


-- 
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: issues-unsubscr...@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to