Repository navigation
Conversation
The sequence encoder checked values in wrapping 64-bit arithmetic and
sign-extended U8/U16/U32, so it emitted sequences Rust refuses to read:
a U8 column [200, 201, 202] got base -56 ("out of range for dtype u8"),
a descending or overflowing U64 got a multiplier whose last value is
"not expressible", and I64 [MIN, MAX] got a wrapped step of -1.
Port Rust's encode_primitive_array: the step is a checked subtraction
and each next value a checked addition in the column's own type, with
unsigned values zero-extended. Found by the mapper module's U64 interop
case (#528); the new cross-check writes each edge with the sequence
encoding forced and has vortex-jni read it back.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Explicit callbacks instead of annotations or reflection (#528): - write: ColumnMapping declares each column once (name, type via the builder method, getter); schema() feeds VortexWriter.create and write(writer, rows) pivots rows into one writeChunk. Unsigned columns take the raw bits of the same-width Java type; nullableXxx twins write null as a null cell. - read: Rows.stream(reader, ScanOptions, Row -> T) maps a projected scan through a Row cursor; cells are addressed by projection position (the scan returns columns in file order, so Row reorders them), and a projected column missing from the file fails fast. Cross-checked against vortex-jni: every column kind written by the mapper is read back by vortex-jni (full, row range, filtered), and a vortex-jni-written wide file is read through small projections. Parked: the read side overlaps the Calcite adapter (rows via SQL) and the Chunk/Array API (speed); see the #528 discussion before merging. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Parked: see the analysis in #528 before merging. The read side (
Rows/Row) overlaps the Calcite adapter and theChunkAPI; the recommendation if resumed is to ship write-only.Two commits, kept separate:
fix: vortex.sequence uses checked arithmetic in the column type: an independent writer bug the mapper's U64 interop case found; Rust refused to read sequences Java wrote for high U8/U16/U32 values, wrapping U64 steps and I64[MIN, MAX]. Ports Rust'sencode_primitive_array(checked arithmetic in the column type, unsigned values zero-extended). New cross-check:JavaWritesRustReadsIntegrationTest#javaWriter_jniReader_sequenceInColumnTypeArithmetic.feat: vortex-mapper module:ColumnMapping(write) andRows/Row(read), explicit callbacks, no reflection or annotations.MapperInteropIntegrationTestcross-checks every column kind against vortex-jni.Refs #528
🤖 Generated with Claude Code