isidentical commented on code in PR #4156:
URL: https://github.com/apache/arrow-datafusion/pull/4156#discussion_r1018530191
##########
datafusion/proto/src/to_proto.rs:
##########
@@ -1188,7 +1188,28 @@ impl TryFrom<&ScalarValue> for protobuf::ScalarValue {
})
}
- datafusion::scalar::ScalarValue::Time64(v) => {
+ // Since the protos only support Time64 and always interpret it to
nanosecond accuracy,
Review Comment:
Question: is there a particular reason why we can't represent these types
directly in our serialization format other than backwards compatibility?
##########
datafusion/common/src/scalar.rs:
##########
@@ -2100,15 +2188,16 @@ impl TryFrom<ScalarValue> for i32 {
}
}
-// special implementation for i64 because of TimeNanosecond
+// special implementation for i64 because of Date64, Time64, Time64 and
Timestamp
Review Comment:
nit: Seems like `Time64` is repeated
```suggestion
// special implementation for i64 because of Date64, Time64, and Timestamp
```
--
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]
