Skip to content

RunEndEncodingDecoder.expandStrings should be lazy, not eager (unbounded n * strLen expansion) #334

Description

@dfa1

Problem

RunEndEncodingDecoder.expandStrings (reader/decode/RunEndEncodingDecoder.java:171-221) fully materializes a run-end-encoded Utf8/Binary column into a VarBinOffsetArray:

long totalBytes = 0;
for (long run = 0; run < numRuns; run++) {
    ...
    totalBytes += count * strLen;
}

MemorySegment outBytes = arena.allocate(totalBytes > 0 ? totalBytes : 1);
MemorySegment outOffsets = arena.allocate((n + 1) * 4L, 4);

This is the same defect #329 fixed for vortex.constant, one encoding over — and structurally worse. RunEnd exists specifically to compress repetition, so this path is by construction a numRuns → n amplification: 1M rows of a single repeated 100-byte string allocates and copies ~100 MB out of ~100 source bytes, plus a 4 MB offsets table.

It is also crash-adjacent. totalBytes is a sum of count * strLen products over untrusted run metadata with no bound applied before arena.allocate. Compare the sibling path in SparseEncodingDecoder.java:311, which explicitly rejects a total outside the value buffer before allocating:

if (totalBytes < 0 || totalBytes > valBytes.byteSize()) {
    throw new VortexException(EncodingId.VORTEX_SPARSE, ...);
}

RunEnd has no equivalent guard, so a crafted file can drive an unbounded allocation (OutOfMemoryError) or, if the products wrap, a negative size straight into arena.allocate — an IllegalArgumentException, not a VortexException (ADR 0003).

Root cause

Every primitive type has a lazy run-end carrier — LazyRunEndLongArray, LazyRunEndIntArray, LazyRunEndShortArray, LazyRunEndByteArray, LazyRunEndBoolArray — all built on the shared RunEndArrays.findRun / RunEndArrays.readRunEnd helpers. VarBin is the only element family with no such carrier, so decode has nothing lazy to return and falls back to physically writing each run's bytes once per row it covers.

Until recently this would also have required editing VarBinArray's permits clause. It no longer does: 7e0d6e7 made VarBinArray non-sealed, so a new representation drops in as an ordinary top-level class in reader.array.

Fix

Add VarBinRunEndArray implements VarBinArray, a near-transcription of LazyRunEndLongArray:

public record VarBinRunEndArray(DType dtype, long length, VarBinArray values, Array runEnds, long offset)
        implements VarBinArray {

    @Override
    public byte[] getBytes(long i) {
        return values.getBytes(RunEndArrays.findRun(runEnds, values.length(), i + offset));
    }
    // getString / getByteLength likewise; forEachByteLength walks runs like walkRuns()
}

Follow the established conventions for a non-contiguous representation: bytesSegment() returns MemorySegment.NULL and segmentIfPresent() returns Optional.empty(), exactly as VarBinChunkedArray / VarBinViewArray / VarBinConstantArray do, so generic consumers still flatten correctly via VarBinArray.toOffsetMode. Then have RunEndEncodingDecoder build it instead of calling expandStrings, and delete expandStrings.

This removes the allocation and the unbounded-product risk outright, rather than bolting a bounds check onto an expansion that should not happen.

docs/compatibility.md's Notes column for vortex.runend should credit the new carrier alongside the LazyRunEndXxxArray family.

Context

Found in a sweep for remaining eager materializations after #329 / 7e0d6e7. This is the largest one left in the reader — see the sibling issues for vortex.sequence, vortex.dict (primitive path), vortex.patched, and fastlanes.delta.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions