comphead commented on code in PR #7225:
URL: https://github.com/apache/datafusion-comet/pull/7225#discussion_r4379457861
##########
spark/src/main/scala/org/apache/spark/sql/comet/util/Utils.scala:
##########
@@ -302,6 +295,22 @@ object Utils extends CometTypeShim with Logging {
}
}
+ /**
+ * Runs `write` on `root` itself when every offset buffer in it starts at
zero, and otherwise on
+ * a slice of it that rebases the offsets to zero and leaves out the bytes
before the first of
+ * them, which Arrow Java would write. The slice shares the other buffers
and holds its own
+ * references on them, so it is closed whether `write` returns or fails, as
it does when a
+ * cancelled broadcast interrupts the task.
+ */
+ private[comet] def withRebasedOffsets[T](root: VectorSchemaRoot)(
+ write: VectorSchemaRoot => T): T = {
+ val written =
+ if (root.getFieldVectors.asScala.forall(offsetsStartAtZero)) root
Review Comment:
This only checks where the offsets start, so the first slice of a list or
map column passes as is. DataFusion's grouped aggregate emits `batch.slice(0,
size)` first, and arrow-rs `GenericListArray::slice` keeps the whole `values`
array, so that slice arrives with offsets from zero and a child vector that the
C Data import sizes to every later slice's elements too. `VectorUnloader` then
writes the child with its own `getValueCount`, so the first cached slice
stores, and `columnSizes` reports, the child of the whole batch. Strings are
not affected, because `BaseVariableWidthVector` stops the data buffer at the
last offset.
Could the `BaseRepeatedValueVector` case also require the last offset to
equal `getDataVector.getValueCount`? `ListVector`'s `splitAndTransfer` already
cuts the child to `[first, last)`, so the existing slice would trim both ends,
which is what arrow-rs's own IPC writer does in `get_list_array_buffers`. The
broadcast path shares the check, so its bytes would shrink too. A head slice
(offset 0, fewer rows than the batch) in the list test would cover it, and the
`offsetsStartAtZero` doc could mention the elements after the slice. I traced
this through the Arrow Java 18.3 and arrow-rs sources and have not run it.
--
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]
