Weijun-H commented on code in PR #8562:
URL: https://github.com/apache/arrow-datafusion/pull/8562#discussion_r1441237668
##########
datafusion/common/src/scalar.rs:
##########
@@ -2936,13 +3026,33 @@ impl fmt::Display for ScalarValue {
)?,
None => write!(f, "NULL")?,
},
- ScalarValue::List(arr)
- | ScalarValue::LargeList(arr)
- | ScalarValue::FixedSizeList(arr) => {
+ ScalarValue::List(arr) => {
+ // ScalarValue List should always have a single element
+ assert_eq!(arr.len(), 1);
+ let options =
FormatOptions::default().with_display_error(true);
+ let formatter =
+ ArrayFormatter::try_new(arr.as_ref() as &dyn Array,
&options)
+ .unwrap();
+ let value_formatter = formatter.value(0);
+ write!(f, "{value_formatter}")?
+ }
+ ScalarValue::LargeList(arr) => {
+ // ScalarValue List should always have a single element
+ assert_eq!(arr.len(), 1);
+ let options =
FormatOptions::default().with_display_error(true);
+ let formatter =
+ ArrayFormatter::try_new(arr.as_ref() as &dyn Array,
&options)
+ .unwrap();
+ let value_formatter = formatter.value(0);
+ write!(f, "{value_formatter}")?
+ }
+ ScalarValue::FixedSizeList(arr) => {
// ScalarValue List should always have a single element
Review Comment:
```suggestion
// ScalarValue FixedSizeList should always have a single
element
```
##########
datafusion/common/src/scalar.rs:
##########
@@ -2936,13 +3026,33 @@ impl fmt::Display for ScalarValue {
)?,
None => write!(f, "NULL")?,
},
- ScalarValue::List(arr)
- | ScalarValue::LargeList(arr)
- | ScalarValue::FixedSizeList(arr) => {
+ ScalarValue::List(arr) => {
+ // ScalarValue List should always have a single element
+ assert_eq!(arr.len(), 1);
+ let options =
FormatOptions::default().with_display_error(true);
+ let formatter =
+ ArrayFormatter::try_new(arr.as_ref() as &dyn Array,
&options)
+ .unwrap();
+ let value_formatter = formatter.value(0);
+ write!(f, "{value_formatter}")?
+ }
+ ScalarValue::LargeList(arr) => {
+ // ScalarValue List should always have a single element
Review Comment:
```suggestion
// ScalarValue LargeList should always have a single element
```
##########
datafusion/proto/src/logical_plan/to_proto.rs:
##########
@@ -1189,24 +1188,79 @@ impl TryFrom<&ScalarValue> for protobuf::ScalarValue {
schema: Some(schema),
};
- match val {
- ScalarValue::List(_) => Ok(protobuf::ScalarValue {
- value: Some(protobuf::scalar_value::Value::ListValue(
- scalar_list_value,
- )),
- }),
- ScalarValue::LargeList(_) => Ok(protobuf::ScalarValue {
- value:
Some(protobuf::scalar_value::Value::LargeListValue(
- scalar_list_value,
- )),
- }),
- ScalarValue::FixedSizeList(_) => Ok(protobuf::ScalarValue {
- value:
Some(protobuf::scalar_value::Value::FixedSizeListValue(
- scalar_list_value,
- )),
- }),
- _ => unreachable!(),
- }
+ Ok(protobuf::ScalarValue {
+ value: Some(protobuf::scalar_value::Value::ListValue(
+ scalar_list_value,
+ )),
+ })
+ }
+ ScalarValue::LargeList(arr) => {
+ // Wrap in a "field_name" column
+ let batch = RecordBatch::try_from_iter(vec![(
+ "field_name",
+ arr.to_owned() as ArrayRef,
+ )])
+ .map_err(|e| {
+ Error::General( format!("Error creating temporary batch
while encoding ScalarValue::List: {e}"))
+ })?;
+
+ let gen = IpcDataGenerator {};
+ let mut dict_tracker = DictionaryTracker::new(false);
+ let (_, encoded_message) = gen
+ .encoded_batch(&batch, &mut dict_tracker,
&Default::default())
+ .map_err(|e| {
+ Error::General(format!(
+ "Error encoding ScalarValue::List as IPC: {e}"
+ ))
+ })?;
+
+ let schema: protobuf::Schema = batch.schema().try_into()?;
+
+ let scalar_list_value = protobuf::ScalarListValue {
+ ipc_message: encoded_message.ipc_message,
+ arrow_data: encoded_message.arrow_data,
+ schema: Some(schema),
+ };
+
+ Ok(protobuf::ScalarValue {
+ value: Some(protobuf::scalar_value::Value::LargeListValue(
+ scalar_list_value,
+ )),
+ })
+ }
+ ScalarValue::FixedSizeList(arr) => {
+ // Wrap in a "field_name" column
+ let batch = RecordBatch::try_from_iter(vec![(
+ "field_name",
+ arr.to_owned() as ArrayRef,
+ )])
+ .map_err(|e| {
+ Error::General( format!("Error creating temporary batch
while encoding ScalarValue::List: {e}"))
Review Comment:
```suggestion
Error::General( format!("Error creating temporary batch
while encoding ScalarValue::FixedSizeList: {e}"))
```
##########
datafusion/proto/src/logical_plan/to_proto.rs:
##########
@@ -1189,24 +1188,79 @@ impl TryFrom<&ScalarValue> for protobuf::ScalarValue {
schema: Some(schema),
};
- match val {
- ScalarValue::List(_) => Ok(protobuf::ScalarValue {
- value: Some(protobuf::scalar_value::Value::ListValue(
- scalar_list_value,
- )),
- }),
- ScalarValue::LargeList(_) => Ok(protobuf::ScalarValue {
- value:
Some(protobuf::scalar_value::Value::LargeListValue(
- scalar_list_value,
- )),
- }),
- ScalarValue::FixedSizeList(_) => Ok(protobuf::ScalarValue {
- value:
Some(protobuf::scalar_value::Value::FixedSizeListValue(
- scalar_list_value,
- )),
- }),
- _ => unreachable!(),
- }
+ Ok(protobuf::ScalarValue {
+ value: Some(protobuf::scalar_value::Value::ListValue(
+ scalar_list_value,
+ )),
+ })
+ }
+ ScalarValue::LargeList(arr) => {
+ // Wrap in a "field_name" column
+ let batch = RecordBatch::try_from_iter(vec![(
+ "field_name",
+ arr.to_owned() as ArrayRef,
+ )])
+ .map_err(|e| {
+ Error::General( format!("Error creating temporary batch
while encoding ScalarValue::List: {e}"))
Review Comment:
```suggestion
Error::General( format!("Error creating temporary batch
while encoding ScalarValue::LargeList: {e}"))
```
--
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]
