parthchandra commented on PR #6371: URL: https://github.com/apache/datafusion-comet/pull/6371#issuecomment-5919288553
Notes on the large-offset narrowing (confirmed it errors cleanly past 2GB rather than silently truncating): - **[+1 to sunchao's P2] `spark/src/main/scala/org/apache/spark/sql/comet/util/Utils.scala:600`** — when the `dataLength > Int.MaxValue` throw fires on, say, the third column of a batch, the narrowed copies already built for the first two are never placed in a `VectorSchemaRoot`, so `serializeBatches` never runs `root.clear()` on them and they leak from `CometArrowAllocator` (same for the materialized `ConstantColumnVector` copies above). This PR widens a pre-existing gap by adding a new throwing conversion. Could `getBatchFieldVectorsWithProviders` close whatever it already built if a later column fails? - **`Utils.scala:575`** (the `narrowOffsets` scaladoc) — a struct or list holding a `large_string` child still serializes with the child's 64-bit offsets untouched, i.e. the same mixed-width schema drift this PR fixes for top-level columns, one level down. Reasonable to scope this PR to the top level, but please file a tracking issue for the nested case and reference it here; nothing tests or errors on it today. - **`Utils.scala:617`** (the validity copy) — the data offsets are rebased by `start`, but validity is copied from bit 0. For an Arrow Java vector that's fine (logical index 0 always sits at validity bit 0 even when `offset[0]` is nonzero). Just confirming no path hands `narrowOffsets` a vector whose validity is bit-offset relative to its first value, which would silently misalign nulls. If it can't happen, a one-line note would help. - **`Utils.scala:623`** (the data copy) — this is a full copy of up to nearly 2 GiB into `CometArrowAllocator` (which no memory budget sees), briefly holding both the original large column and the narrowed copy. It matches the existing `ConstantColumnVector` path, so no change requested - just confirming the transient peak is understood and acceptable. -- 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]
