alamb commented on code in PR #23192:
URL: https://github.com/apache/datafusion/pull/23192#discussion_r3476697762


##########
datafusion/functions-nested/src/array_compact.rs:
##########
@@ -130,14 +130,22 @@ fn compact_list<O: OffsetSizeTrait>(
     field: &Arc<arrow::datatypes::Field>,
 ) -> Result<ArrayRef> {
     let values = list_array.values();
+    // Use logical nulls so element types without a validity buffer
+    // (e.g. NullArray) are still treated as null.
+    let values_null_count = values.logical_null_count();

Review Comment:
   I think this might call `logical_nulls under the coverws -- it might make 
sense to just call `logical_nulls()` here and then use that directly rather 
than having to call logical_nulls again below
   
   ```rust
       let values_nulls = values
           .logical_nulls()
           .expect("non-zero logical_null_count implies logical_nulls is Some");
   ```



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

Reply via email to