fix: decode fastlanes.rle over F64/F32 (#209)#214
Merged
Merged
Conversation
5 tasks
RleEncodingDecoder threw "fastlanes.rle: unsupported ptype F64" because its ptype switch only had integer arms. The Python Vortex writer (vortex-data 0.69.0) RLE-encodes double columns with long constant runs, so such files could not be read. Per the Rust reference (encodings/fastlanes/src/rle/vtable/mod.rs), the RLE value child is DType::Primitive(dtype.as_ptype(), NonNullable) — i.e. the same ptype as the array, stored raw. F64/F32 therefore serialize identically to the integer arms: a values pool (8-/4-byte elements), a u8/u16 indices child, and per-chunk values_idx_offsets. Nullability is delegated to the indices child (ValidityVTable::validity slices indices and reads its validity), matching how the existing arms already propagate a MaskedArray from a nullable indices child; the new F64/F32 arms reuse that same wrapping. Added LazyRleDoubleArray / LazyRleFloatArray (mirroring LazyRleLongArray's chunk-walk decode) and the F64/F32 decode + empty arms. Buffers stay zero-copy; the value-pool reads use branch-split broadcast handling like the existing arms. Verified on the real seoul-bike-sharing file (F64 weather columns): the CLI export now completes and all 8760 rows of temperature and wind_speed match the parquet oracle exactly. Closes #209 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add RleF64InteropIntegrationTest: the JNI (Rust) writer compresses long constant runs of doubles to fastlanes.rle, so this drives the real decode path (F64/F32 arms, readDoubles/readFloats over a real segment, real valuesIdxOffsets) end to end rather than hand-built lazy records. Each case inspects the file via VortexInspector and assumeTrue it contains fastlanes.rle before comparing values, so it never asserts on an unintended encoding. The JNI compressor does choose RLE for the crafted 512-row constant runs: all three cases (F64, F32, nullable F64) run, none skip. The nullable case decodes without error and checks non-null values exactly; it deliberately does not assert the null mask, since surfacing RLE validity is the separate in-flight fix #210 (matching jniWriter_nullableColumn_decodesWithoutError). Also address review nits on LazyRleArrayTest: replace inline FQNs with imports, add the missing // When / // Then markers, and add a NaN/infinity bit-preservation case for the double arm. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
Reviewed (vortex-reviewer agent): code verified as a correct mirror of the integer arms (chunk-walk math, broadcast handling, float bit-preservation, validity flow all checked; 30+ unit tests executed at the PR ref). The review's one should-fix — a missing ground-truth interop test — is addressed in 31ec02f: |
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.
Fixes #209, found by the Raincloud conformance suite (#205): the Python writer (
vortex-data 0.69.0) RLE-encodes double columns (weather measurements with long constant runs) andRleEncodingDecoderthrewfastlanes.rle: unsupported ptype F64.Per the Rust reference (
encodings/fastlanes/src/rle/vtable/mod.rs), F64/F32 RLE serializes identically to the integer arms — a raw value pool child, indices, and per-1024-row chunk offsets; the values child dtype is the array's own ptype. The fix addsLazyRleDoubleArray/LazyRleFloatArraymirroringLazyRleLongArray, plus F64/F32 decode arms with the sameSegmentBroadcasthandling as the existing readers. Validity was already correct here: the decoder mirrors Rust's delegation to the indices child and re-wrapsMaskedArray, and the new arms flow through unchanged.Verified on the real corpus:
uci-seoul-bike-sharing-demand(8,760 rows) exports fully; full-column compares oftemperatureandwind_speed(both F64 RLE) match the Parquet oracle exactly. Reader suite: 1091 tests green.Merge notes: independent of #212/#213, any order; trivial CHANGELOG conflict. After merge, the conformance matrix flips
uci-seoul-bike-sharing-demand → okin a coordination commit.Closes #209
🤖 Generated with Claude Code