feat(java): add fragment-level addColumns from an Arrow stream - #9231
zhangyue19921010 wants to merge 1 commit into
Conversation
Expose FileFragment::add_columns(NewColumnTransform::Reader) through the Java bindings as Fragment.addColumns(ArrowArrayStream, Optional<Long> batchSize), the fragment-level equivalent of Dataset.addColumns(reader). Unlike Fragment.mergeColumns, the stream is zipped positionally against the fragment's live rows and is never buffered in full, so callers that can supply one row per live row in row address order (e.g. distributed backfill engines) can add columns far larger than memory. Streams with too few or too many rows fail instead of misaligning.
github-actions
Bot
added
A-java
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The fragment-scoped streaming path is the right mechanism and the normal row-alignment behavior works, but the new batch-size boundary must reject zero before entering Rust's updater. A positive-value check keeps invalid caller configuration from terminating the JVM while preserving the streaming design.
| let batch_size = match env.get_long_opt(&batch_size)? { | ||
| Some(value) => Some( | ||
| value | ||
| .try_into() |
There was a problem hiding this comment.
A zero value passes this conversion and reaches FragmentReader::read_all(0), whose system-column path constructs step_by(0). The panic crosses the extern "system" entrypoint, cannot unwind, and aborts the JVM instead of returning an input error. Reject non-positive values here (or at the shared updater boundary) and cover Optional.of(0L) with a Java regression test.
Reproducer
After exporting a valid 21-row stream in testAddColumnsByReader, I ran:
assertThrows(
IllegalArgumentException.class,
() -> fragment.addColumns(stream, Optional.of(0L)));Expected: IllegalArgumentException. Observed:
panicked ... step_by.rs: assertion failed: step != 0
panic in a function that cannot unwind
thread caused non-unwinding panic. aborting.
lance-gatekeeper
Bot
added
the
K-changes
|
blocked on #9233 |
lance-gatekeeper
Bot
removed
the
K-changes
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The author's dependency note changes the acceptance path: #9233 contains the shared Updater::try_new guard for the verified zero-batch-size JVM abort. The fragment-scoped streaming design remains sound, but land #9233 before this PR (and refresh this branch if needed); until that guard is in this PR's merge base, the current head remains unsafe for Optional.of(0L).
lance-gatekeeper
Bot
added
the
K-changes
1 participant
Expose FileFragment::add_columns(NewColumnTransform::Reader) through the Java bindings as Fragment.addColumns(ArrowArrayStream, Optional batchSize), the fragment-level equivalent of Dataset.addColumns(reader).
Unlike Fragment.mergeColumns, the stream is zipped positionally against the fragment's live rows and is never buffered in full, so callers that can supply one row per live row in row address order (e.g. distributed backfill engines) can add columns far larger than memory. Streams with too few or too many rows fail instead of misaligning.