andygrove opened a new pull request, #7504: URL: https://github.com/apache/datafusion-comet/pull/7504
## Which issue does this PR close? Closes #7494. ## Rationale for this change With `spark.comet.exec.columnarToRow.native.enabled=true` (off by default), the native conversion writes all of a batch's rows into one buffer and returned each row's start as an `i32`. Once a batch's rows took more than 2 GiB, the offsets wrapped negative without any error, and the JVM read the remaining rows from memory before the buffer. That gives garbage rows, or a SIGSEGV if the memory is unmapped. Every column can be well under Arrow's limit when this happens: two string columns averaging 150 KB per value take about 2.4 GB of rows in an 8192-row batch. Spark converts one row at a time, so it has no such limit. This is one of the offset overflow sites tracked in #7267. ## What changes are included in this PR? - `ColumnarToRowContext` keeps row offsets as `i64`, and `columnarToRowConvert` passes them to the JVM as a `long[]`. `NativeColumnarToRowInfo.offsets` is now a `long[]`, which `NativeRowIterator` adds to the buffer address as before. - Row lengths stay `i32` but are now checked. A row larger than Spark's `ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH` fails with `Row too large for UnsafeRow conversion`, the limit `DirectColumnarToRowConverter` already applies. Spark can't build a row that large either. ## How are these changes tested? - `test_row_offsets_past_2_gib` converts three rows of eight string columns that share one array, so the third row starts past `i32::MAX`, and checks the offsets, the lengths and the third row's contents. It needs about 4 GB of memory, so it is `#[ignore]`d. It passes locally with `--ignored`, and fails when the offset is truncated to 32 bits again. - `test_checked_row_length` checks the length limit at and past the maximum, including sizes that a plain `as i32` cast would wrap. It fails when the check is removed. - The existing `columnar_to_row` and `jni_api` tests pass, and `CometNativeColumnarToRowSuite` (26 tests, default Spark 4.1 profile) passes against a library built from this branch, which exercises the new JNI constructor signature. -- 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]
