Skip to content

ConstantEncodingDecoder.decodeString should be lazy, not eager #329

Description

@dfa1

Problem

Every other vortex.constant value type has a LazyConstantXxxArray (LazyConstantLongArray, LazyConstantByteArray, LazyConstantBoolArray, LazyConstantDecimalArray, …): a metadata-only record holding a single value that every getXxx(i) returns directly — O(1), no buffer allocated regardless of row count.

ConstantEncodingDecoder.decodeString (Utf8/Binary) never got the same treatment. It eagerly materializes the constant string into a real VarBinArray.OffsetMode:

MemorySegment bytesSeg = ctx.arena().allocate(n * strLen);
for (long i = 0; i < n; i++) {
    MemorySegment.copy(MemorySegment.ofArray(strBytes), 0L, bytesSeg, i * strLen, strLen);
}

MemorySegment offsetsSeg = ctx.arena().allocate((n + 1) * 4L, 4);
for (long i = 0; i <= n; i++) {
    offsetsSeg.setAtIndex(VortexFormat.LE_INT, i, (int) (i * strLen));
}

This is O(n) allocation + copy for a value that's the same on every row, inconsistent with every sibling constant type, and the n * strLen multiplication has no overflow guard — a crafted file combining a large rowCount (from the layout, itself currently unvalidated — separate concern) with a large scalar string could wrap n * strLen to a negative long, which ctx.arena().allocate(negative) turns into a raw IllegalArgumentException instead of VortexException (ADR 0003), or a huge-but-positive product just OOMs.

Root cause

VarBinArray's only flat representation, OffsetMode, hard-requires a real n+1-entry offsets table and n-rows-worth of bytes — there's no broadcast/modulo path in it the way AbstractMaterializedArray gives the primitive Materialized*Arrays (i % elementCount). So decodeString has no lazy shape available and falls back to physically writing the same string n times just to produce something OffsetMode-shaped.

Fix

Add a LazyConstantVarBinArray implements VarBinArray (or equivalent) holding the string/bytes once, with getBytes/getString/getByteLength returning the same value for any i — mirroring LazyConstantByteArray et al. Switch ConstantEncodingDecoder.decodeString to build it instead of the eager OffsetMode. This removes the allocation (and the overflow risk) entirely rather than just bounds-checking the multiplication.

Context

Surfaced during a subagent review of # (RunEnd/Constant/Zoned/Pco adversarial-input hardening, branch security/adversarial-tests-batch2). Not fixed in that batch — it's a design gap (missing lazy representation), not a crash, and the batch's decodeDecimal fix already closed the actual crash risk (bytes_value() null NPE) for the Constant encoding's TODO item.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions