andygrove commented on code in PR #4934:
URL: https://github.com/apache/datafusion-comet/pull/4934#discussion_r3603777093
##########
native/spark-expr/src/array_funcs/array_compact.rs:
##########
@@ -117,6 +117,22 @@ fn compact_list<OffsetSize: OffsetSizeTrait>(
};
let values = list_array.values();
+
+ // logical_nulls() (not nulls()) is used so a NullArray, whose elements are
+ // all logically null, is correctly reported as fully null.
NullArray::nulls()
+ // returns None, which would make is_null() report false for every element.
+ let value_nulls = values.logical_nulls();
Review Comment:
Switched the fast-path guard to `values.logical_null_count() == 0`. The
`logical_nulls()` clone is only materialized on the slow path where it is
actually consumed, so Dictionary/Run/Null values children no longer pay an O(n)
allocation on this path.
##########
native/spark-expr/src/array_funcs/array_compact.rs:
##########
@@ -117,6 +117,22 @@ fn compact_list<OffsetSize: OffsetSizeTrait>(
};
let values = list_array.values();
+
+ // logical_nulls() (not nulls()) is used so a NullArray, whose elements are
Review Comment:
Added `null_array_values_child_all_rows_empty` (builds `List<Null>` with a
`NullArray` values child) and asserts every row compacts to length 0.
--
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]
