alamb commented on code in PR #2840:
URL: https://github.com/apache/arrow-datafusion/pull/2840#discussion_r918246905
##########
datafusion/common/src/scalar.rs:
##########
@@ -1330,8 +1330,10 @@ impl ScalarValue {
Some(scalar_vec)
}
};
- let data_type = nested_type.data_type().clone();
- ScalarValue::List(value, Box::new(data_type))
+ ScalarValue::List(
+ value,
+ Box::new(Field::new("item",
nested_type.data_type().clone(), true)),
Review Comment:
this is effectively going to lose the original field name (which isn't
attached to the `array`) in this particular codepath, which is a similar
problem to what I observed in
https://github.com/apache/arrow-datafusion/issues/2874
However, this PR certainly seems better than the current state of master
which loses the field name through all paths
##########
datafusion/proto/src/from_proto.rs:
##########
@@ -656,21 +658,22 @@ impl TryFrom<&protobuf::ScalarListValue> for ScalarValue {
fn try_from(
scalar_list_value: &protobuf::ScalarListValue,
) -> Result<Self, Self::Error> {
- use protobuf::{scalar_type::Datatype, PrimitiveScalarType};
+ if 2 > 1 {
Review Comment:
Maybe it is because the tests don't have coverage of `DataType::List`
This is a good experiment (add in a `panic`) but we probably need to remove
this prior to merging this PR
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
