jeremybarner opened a new pull request, #1804:
URL: https://github.com/apache/iceberg-go/pull/1804
Production crash chain (exit code 2, unrecoverable panic):
1. pqarrow builds *array.Binary (int32 offsets) for every Parquet BYTE_ARRAY
column. BinaryBuilder.appendNextOffset guards the int32 cast only with
debug.Assert, which is an empty function under the default !assert build tag —
inert in every production binary. No other guard exists.
2. A single read batch (default 131072 rows) whose aggregate BYTE_ARRAY
payload exceeds 2^31-1 bytes (~16.4 KB average per row) wraps int32 silently
and writes negative offsets into the offset buffer.
3. compute.FilterRecordBatch on a deletion-vector column does not use the
Filter kernel for binary types — it converts the Boolean keep-mask to row
indices via GetTakeIndices then dispatches to TakeExec(VarBinaryImpl[int32])
(arrow/compute/internal/kernels/vector_selection.go:1703). That function does
rawData[valOffset : valOffset+valSize] with no bounds check. A negative
valOffset sign-extends to a huge uint64 and panics with "slice bounds out of
range [:-2000000000]" inside a goroutine Arrow's compute executor owns —
nothing can recover it.
Fix: (*ParquetFileSource).GetReader now calls arrProps.SetForceLarge(i,
true) for every physical leaf column before constructing the
pqarrow.FileReader. This forces LargeBinary / LargeString (int64 offsets) for
all BYTE_ARRAY columns, making overflow structurally impossible at the cost of
8-byte vs 4-byte offset entries with no data copy.
Blast radius: none. ToRequestedSchema with useLargeTypes=false (the default)
runs as the last pipeline step and casts LargeBinary/LargeString back to
Binary/String, so callers see the same types as before. Dict-encoded columns
(SetReadDict=true, used for the positional-delete file_path column) are
unaffected: pqarrow wraps the primitive in DictionaryType before the
IsBinaryLike check, so SetForceLarge is a no-op for those columns.
Also hardens filterByDeletionVector with two guards it previously lacked:
- Zero-row batches pass through without advancing the absolute-position
counter, so subsequent batches stay aligned with the keep-mask.
- If the Parquet reader delivers more rows than the file metadata claims
(task.Value.File.Count()), the function now returns a descriptive error instead
of panicking with an out-of-bounds slice in array.NewSlice.
RESIDUAL UNCERTAINTY: the overflow requires ~16.4 KB average payload per row
in a single 131072-row batch. That is plausible for a data-dense monday.com
board but has NOT been confirmed against the specific account that crashed. The
reproduction is synthetic (hand-crafted negative offsets rather than a real 2
GiB batch). The fix is still correct as hardening.
Tests added (dv_scanner_binary_panic_regression_test.go):
- TestBinaryOffsetCorruptionPanicChain: subprocess reproduces the exact
panic ("slice bounds out of range [:-2000000000]" at vector_selection.go:1703).
Outer test asserts subprocess exits non-zero. Confirms the mechanism.
- TestDVScanWithBinaryColumn: end-to-end DV scan with a real Parquet
BYTE_ARRAY file (4 subtests: no DV, interior deletes, all-rows deleted,
boundary deletes). First var-binary DV coverage in the fork; this gap let the
overflow ship.
- TestFilterByDeletionVectorZeroRowBatch: empty batch passes through without
advancing nextIdx; subsequent batches remain correctly aligned.
- TestFilterByDeletionVectorRowCountGuard: reader delivering more rows than
metadata claims returns a descriptive error instead of panicking.
--
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]
