kbendick commented on a change in pull request #1455:
URL: https://github.com/apache/iceberg/pull/1455#discussion_r487772364
##########
File path:
mr/src/main/java/org/apache/iceberg/mr/hive/serde/objectinspector/IcebergDateObjectInspector.java
##########
@@ -42,17 +42,17 @@ private IcebergDateObjectInspector() {
@Override
public Date getPrimitiveJavaObject(Object o) {
- return o == null ? null : Date.valueOf((LocalDate) o);
+ return o == null ? null : Date.valueOf(o.toString());
}
@Override
- public DateWritable getPrimitiveWritableObject(Object o) {
- return o == null ? null : new
DateWritable(DateTimeUtil.daysFromDate((LocalDate) o));
+ public DateWritableV2 getPrimitiveWritableObject(Object o) {
+ return o == null ? null : new
DateWritableV2(DateTimeUtil.daysFromDate((LocalDate) o));
}
@Override
public Object copyObject(Object o) {
- return o == null ? null : new Date(((Date) o).getTime());
+ return o == null ? null : Date.valueOf(o.toString());
Review comment:
Are you sure that `toString` is the appropriate method to call here to
get the time from the date?
`toString` is usually unsafe in these situations, especially for a function
that takes in `Object`. At the least, if an object were passed in previously
that could not be casted to `java.sql.Date`, then the user would get a decent
stack trace explaining that to them.
To me, it seems like the stack trace's value to developers has decreased
here, but perhaps I'm not understanding the semantics of
`org.apache.hadoop.hive.common.type.Date.valueOf`.
Additionally, I think we tend to prefer to use `java.sql.Date` as it is the
more portable representation across the many query engines that need to be
supported. Additionally, I believe that `java.sql.Date` is expressed in the
documentation as the physical type for a logical Date. Again, somebody please
correct me if I'm wrong.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
