sarutak commented on code in PR #7663:
URL: https://github.com/apache/arrow-datafusion/pull/7663#discussion_r1343032874


##########
datafusion/core/src/datasource/avro_to_arrow/arrow_array_reader.rs:
##########
@@ -775,14 +776,20 @@ impl<'a, R: Read> AvroArrowArrayReader<'a, R> {
                         let len = rows.len();
                         let num_bytes = bit_util::ceil(len, 8);
                         let mut null_buffer = 
MutableBuffer::from_len_zeroed(num_bytes);
+                        let empty_vec = vec![];
                         let struct_rows = rows
                             .iter()
                             .enumerate()
                             .map(|(i, row)| (i, 
self.field_lookup(field.name(), row)))
                             .map(|(i, v)| {
+                                let v = v.map(maybe_resolve_union);
                                 if let Some(Value::Record(value)) = v {
                                     bit_util::set_bit(&mut null_buffer, i);
                                     value
+                                } else if v.is_none() {

Review Comment:
   Now we have multiple branches here, is it pretty to rewrite with `match` 
expression?
   Also, if `v.is_none()`, it should do `panic!` because this case means that 
field lookup fails. It doesn't  mean `Null`.



##########
datafusion/core/src/datasource/avro_to_arrow/arrow_array_reader.rs:
##########
@@ -97,15 +97,15 @@ impl<'a, R: Read> AvroArrowArrayReader<'a, R> {
         schema_lookup: &'b mut BTreeMap<String, usize>,
     ) -> Result<&'b BTreeMap<String, usize>> {
         match schema {
-            AvroSchema::Record(RecordSchema {
-                name,
-                fields,
-                lookup,
-                ..
-            }) => {
+            AvroSchema::Union(union_schema) => {
+                if union_schema.is_nullable() {
+                    let rec_schema = &union_schema.variants()[1];

Review Comment:
   This part seems incorrect. The second variants is not always non-null, but 
the first one can be.



-- 
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]

Reply via email to