alamb commented on code in PR #3612:
URL: https://github.com/apache/arrow-datafusion/pull/3612#discussion_r981209205
##########
datafusion/proto/proto/datafusion.proto:
##########
@@ -739,11 +742,19 @@ message IntervalMonthDayNanoValue {
int64 nanos = 3;
}
+message StructValue {
+ // encode null explicitly to distinguish a struct with no fields (is
+ // that possible?) from a null value
+ bool is_null = 1;
+ repeated ScalarValue field_values = 2;
+ repeated Field fields = 3;
+}
message ScalarValue{
oneof value {
- // Null value of any type (type is encoded)
- PrimitiveScalarType null_value = 19;
+ // was PrimitiveScalarType null_value = 19;
+ // Null value of any type
+ ArrowType null_value = 33;
Review Comment:
You mean why are nulls typed in general? Basically because of how
`ScalarValue::` is implemented (as an `Option<>` around the underlying native
type). I think @Jimexist tried to clean it up at some point and make
`ScalarValue::None` and then all the variants like `ScalarValue::Int8` have
values like `i8` rather than `Option<i8>`.
I can't remember what the problem was but it didn't work easily.
In my opinion at least the serialization should follow how they are
implemented in `ScalarValue` and if we improve `ScalarValue` then we can also
improve the serialization code
--
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]
