martin-g commented on code in PR #18809:
URL: https://github.com/apache/datafusion/pull/18809#discussion_r2540787525
##########
datafusion/datasource-avro/src/avro_to_arrow/arrow_array_reader.rs:
##########
@@ -899,7 +929,27 @@ fn flatten_values<'a>(values: &[&'a Value]) -> Vec<&'a
Value> {
/// This is useful for interpreting any Avro array as string, dropping nulls.
Review Comment:
```suggestion
/// This is useful for interpreting any Avro array as bytes, dropping nulls.
```
##########
datafusion/datasource-avro/src/avro_to_arrow/arrow_array_reader.rs:
##########
@@ -899,7 +929,27 @@ fn flatten_values<'a>(values: &[&'a Value]) -> Vec<&'a
Value> {
/// This is useful for interpreting any Avro array as string, dropping nulls.
/// See `value_as_string`.
#[inline]
-fn flatten_string_values(values: &[&Value]) -> Vec<Option<String>> {
+fn flatten_binary_values<'a>(values: &[&'a Value]) -> Vec<Option<Cow<'a,
[u8]>>> {
+ values
+ .iter()
+ .flat_map(|row| {
+ let row = maybe_resolve_union(row);
+ if let Value::Array(values) = row {
+ values.iter().map(resolve_bytes).collect::<Vec<Option<_>>>()
+ } else if let Value::Null = row {
+ vec![]
+ } else {
+ vec![resolve_bytes(row)]
+ }
+ })
+ .collect::<Vec<Option<_>>>()
+}
+
+/// Flattens a list into string values, dropping Value::Null in the process.
+/// This is useful for interpreting any Avro array as string, dropping nulls.
+/// See `value_as_string`.
Review Comment:
```suggestion
/// See `resolve_string`.
```
##########
datafusion/datasource-avro/src/avro_to_arrow/arrow_array_reader.rs:
##########
Review Comment:
```suggestion
/// Flattens a list into byte values, dropping Value::Null in the process.
```
##########
datafusion/datasource-avro/src/avro_to_arrow/arrow_array_reader.rs:
##########
@@ -899,7 +929,27 @@ fn flatten_values<'a>(values: &[&'a Value]) -> Vec<&'a
Value> {
/// This is useful for interpreting any Avro array as string, dropping nulls.
/// See `value_as_string`.
Review Comment:
```suggestion
/// See `resolve_bytes`.
```
##########
datafusion/datasource-avro/src/avro_to_arrow/arrow_array_reader.rs:
##########
@@ -921,14 +971,13 @@ fn flatten_string_values(values: &[&Value]) ->
Vec<Option<String>> {
/// Reads an Avro value as a string, regardless of its type.
/// This is useful if the expected datatype is a string, in which case we
preserve
/// all the values regardless of they type.
Review Comment:
```suggestion
/// all the values regardless of their type.
```
##########
datafusion/datasource-avro/src/avro_to_arrow/arrow_array_reader.rs:
##########
@@ -359,13 +355,15 @@ impl<R: Read> AvroArrowArrayReader<'_, R> {
let builder =
builder.as_any_mut().downcast_mut::<ListBuilder<StringDictionaryBuilder<D>>>().ok_or_else(||SchemaError(
"Cast failed for
ListBuilder<StringDictionaryBuilder> during nested data parsing".to_string(),
))?;
- for val in vals {
+ vals.into_iter().try_for_each(|val| {
if let Some(v) = val {
- let _ = builder.values().append(&v)?;
+ builder.values().append(v)?;
} else {
- builder.values().append_null()
- };
- }
+ builder.values().append_null();
+ }
+
+ Result::<_, ArrowError>::Ok(())
Review Comment:
This looks fishy.
`Result` is `datafusion_common::error::Result`
https://github.com/EmilyMatt/datafusion/blob/0ea75919a3b373b6719601cd69474d14f377e319/datafusion/datasource-avro/src/avro_to_arrow/arrow_array_reader.rs#L49
but it is a type alias with `E = DataFusionError`
https://github.com/apache/datafusion/blob/0ea75919a3b373b6719601cd69474d14f377e319/datafusion/common/src/error.rs#L61
How does this compile ?!
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
