andygrove commented on code in PR #6238:
URL: https://github.com/apache/datafusion-comet/pull/6238#discussion_r4173395647
##########
native/core/src/execution/operators/iceberg_write.rs:
##########
@@ -1133,6 +1155,44 @@ fn contains_float(data_type: &DataType) -> bool {
}
}
+/// `true` when `array` holds a float or double under a list or map whose
child array reaches
+/// past the rows `array` covers. Slicing a list or map narrows only its
offsets and leaves the
+/// child whole, and the NaN-count visitor walks the whole child. A struct's
children are sliced
+/// with it, so a struct only matters for the lists and maps inside it. A
container this does not
+/// inspect is treated as reaching past whenever it holds a float, which errs
toward gathering.
+fn floats_outside_window(array: &dyn Array) -> bool {
+ match array.data_type() {
+ DataType::List(field) => {
+ let list = array.as_list::<i32>();
+ contains_float(field.data_type())
+ && reaches_past(list.value_offsets(), list.values().as_ref())
+ }
+ DataType::LargeList(field) => {
Review Comment:
Agreed, `schema_to_arrow_schema` only builds `List` and `Map`, so that arm
could never fire. I dropped it in 9945888fbb, and `reaches_past` now takes
`&[i32]`.
##########
native/core/src/execution/operators/iceberg_write.rs:
##########
@@ -972,6 +974,11 @@ impl ClusteredBatchSplitter {
/// safe, because `StructArray::slice` slices them, so the only schemas that
need the fix are the
/// ones with a float or double under a list or map; those ranges go through
`take`, which gathers
/// the referenced children into fresh compacted arrays.
+///
+/// A range that covers its whole batch is handed on as-is, which is exact
only if the batch itself
Review Comment:
There isn't one yet. I'm writing it up for iceberg-rust and will link it
here, with a comment next to `compact`, once it's filed. The visitor reads
`values()` and `entries()` without the parent's offsets or validity, so the
same upstream fix would also cover #6562, the NULL-entry gap from
parthchandra's comment.
--
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]
