[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #7002: avoid copying listarray in unset exec

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on code in PR #7002:
URL: https://github.com/apache/arrow-datafusion/pull/7002#discussion_r1269813166


##########
datafusion/core/src/physical_plan/unnest.rs:
##########
@@ -236,21 +236,21 @@ fn build_batch(
     match list_array.data_type() {
         DataType::List(_) => {
             let list_array = list_array.as_any().downcast_ref::<ListArray>().unwrap();
-            unnest_batch(batch, schema, column, &list_array)
+            unnest_batch(batch, schema, column, &list_array, list_array.values())

Review Comment:
   Yes, this makes sense



##########
datafusion/core/src/physical_plan/unnest.rs:
##########
@@ -263,43 +263,27 @@ fn unnest_batch<T>(
     schema: &SchemaRef,
     column: &Column,
     list_array: &T,
+    list_array_values: &Arc<dyn Array>,
 ) -> Result<RecordBatch>
 where
     T: ArrayAccessor<Item = ArrayRef>,
 {
-    // Create an array with the unnested values of the list array, given the list
-    // array:
-    //
-    //   [1], null, [2, 3, 4], null, [5, 6]
-    //
-    // the result array is:
-    //
-    //   1, null, 2, 3, 4, null, 5, 6
-    //
-    let unnested_array = unnest_array(list_array)?;
-
-    // Create an array with the lengths of each list value in the nested array.
-    // Given the nested array:
-    //
-    //   [1], null, [2, 3, 4], null, [5, 6]
-    //
-    // the result array is:
-    //
-    //   1, null, 3, null, 2
-    //
-    // Depending on the list type the result may be Int32Array or Int64Array.
     let list_lengths = list_lengths(list_array)?;
 
     // Create the indices for the take kernel and then use those indices to create
     // the unnested record batch.
     match list_lengths.data_type() {
         DataType::Int32 => {
             let list_lengths = as_primitive_array::<Int32Type>(&list_lengths)?;
+            let unnested_array =
+                unnest_array(list_array, list_array_values, list_lengths)?;

Review Comment:
   I am still struggling to understand the need to call `unnest_array` and why the take indexes can't be calculated directly against the original values array.
   
   



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org