Repository navigation
fix: native fixes the FileFormat connector needs (nested over-read, timestamp range, schema export leak) - #34
Open
gitRasheed wants to merge 6 commits into
Open
gitRasheed wants to merge 6 commits into
gitRasheed wants to merge 6 commits into
Conversation
… rest of the field
A nullable struct expands a field access `$.x` to `mask($.x, is_not_null($))`. For a
path below the field, such as `$.x.q`, the mask then sits between the path and the
field, so the partitioner hands the field's reader a bare `$` and the whole of `x`
is read. Mask the whole path instead.
On a file with `t: {x: {p, q}, r}`, both nullable, reading `$.t.x.q` or filtering on
`$.t.x.q > n` read 6.0 MB before and 160 KB after.
Signed-off-by: gitRasheed <99349713+gitRasheed@users.noreply.github.com>
Timestamp scalars were validated against `jiff::Timestamp`'s range, which stops 26 hours short of the end of year 9999, so a value Arrow, Parquet and Spark allow, such as `9999-12-31T00:00:00Z`, could neither be used as a literal nor written: the file statistics built the min/max scalar with `Scalar::extension_ref` and panicked. Every `i64` count of the unit is a valid timestamp; jiff only bounds what `Display` can render, so drop the range check, fall back to the raw count when the calendar rendering is unavailable, and build the min/max scalars fallibly so a rejected extreme is an error rather than a panic. Signed-off-by: gitRasheed <99349713+gitRasheed@users.noreply.github.com>
Now that timestamps beyond jiff's range can be written, `split_temporal` could see second and millisecond counts more than i32::MAX days from the epoch. `split_with_divisor` truncated the days to i32 with `as_()`, so such values silently wrapped when compressed: `i64::MAX` seconds round-tripped as 50027779063807. The compress comment still claimed jiff validation made the narrowing safe, the btrblocks TemporalScheme applied the encoding with no range check, and `try_extract_days_constant` used the same truncating split, so `dtp <= lit` with such a literal was rewritten to a wrapped days constant. Microsecond and nanosecond counts cannot overflow the days part. `timestamp::split` now errors when the days do not fit, so the days pushdown declines the constant and the compare kernel falls back to the canonical comparison. `split_temporal` checks the valid min/max of second and millisecond arrays and errors instead of wrapping, and the new `can_split_temporal` lets the btrblocks TemporalScheme skip such arrays so the storage array is compressed and the data round-trips exactly. The narrowing compare test built its fixture from `i64::MAX` seconds and relied on the wrap; it now uses a value above i32::MAX whose days fit. Signed-off-by: gitRasheed <99349713+gitRasheed@users.noreply.github.com>
…hold the literal Compression narrows the days part, for example to u16 for dates before 2149. A midnight literal past that range, such as 9999-12-31, made the comparison push-down cast its days to the narrowed type and fail with "Cannot cast i32 to u16". The rule now declines, and the comparison runs on the full timestamps. Signed-off-by: gitRasheed <99349713+gitRasheed@users.noreply.github.com>
Signed-off-by: gitRasheed <99349713+gitRasheed@users.noreply.github.com>
build() closed the exported ArrowSchema without calling its release callback. Native code only borrows the schema, so the child structs the export allocated were never freed, and every writer's allocator closed with outstanding memory. Signed-off-by: gitRasheed <99349713+gitRasheed@users.noreply.github.com>
This was referenced Oct 29, 2026
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
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.
Summary
Native fixes that vortex-data#9658's connector needs and upstream hasn't merged yet. Part of #31. Stacked on #35, the vortex-data#9658 integration branch.
Changes
VortexWriter.build()releases its schema export. Vortex spark integration is a fileformat not a tableprovider vortex-data/vortex#9658's writer still builds aVortexWriterper file.These are cherry-picks of the upstream PRs as they stand; if those change in review, this branch follows.
Testing
The tests come with each commit. Checked locally:
cargo test -p vortex-layout -p vortex-datetime-parts --lib, and./gradlew :vortex-jni:check :vortex-spark-4.0_2.13:check.I used AI tools to help write and review this change.