Repository navigation
Preserve Arrow round-trip types with typed extension plugins - #11360
Conversation
Merging this PR will not alter performance2 warnings
Comparing Footnotes
|
| DataType::LargeUtf8 => to_arrow_byte_array::<LargeUtf8Type>(array, ctx), | ||
| DataType::BinaryView => to_arrow_byte_view::<BinaryViewType>(array, options, ctx), | ||
| DataType::Utf8View => to_arrow_byte_view::<StringViewType>(array, options, ctx), | ||
| // TODO(joe): pass down preferred |
There was a problem hiding this comment.
Note to self - these todos are stale
robert3005
left a comment
There was a problem hiding this comment.
The arrow parts looks good, we can make the runend fallback better
a4d1b8e to
d06facb
Compare
d06facb to
dd7ebb1
Compare
dd7ebb1 to
8e976e6
Compare
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
8e976e6 to
0607c3b
Compare
## Summary Follow-up to #11375 and [the DuckDB exporter review](#11360 (comment)). Chunked run-end inputs currently lose their cached dictionary when an output batch crosses a chunk boundary. Exposing the existing batch codes through `DictionaryParts` lets those ranges reuse the values dictionary, including ranges shared with ordinary dictionary chunks. ## Changes Keep the run-end specialization because routing through `to_dict()` would make the generic exporter materialize the entire codes array. The existing fill loop also remains: using `slice_relative_ends` increased standalone single-row-run export time in all five trial pairs (median paired ratio 1.064). Empty inputs now avoid run lookup and values conversion, and zero-length requests avoid scalar access. Constant batches include requests ending exactly at a run boundary. Compact inline regressions cover empty requests, slice offsets, exact run boundaries, nulls, successive batches, and shared or different dictionaries across chunks. The existing null and chunked-values tests remain. Measurements on `702fd58dea`, before the test and module cleanup, showed about 15% lower batch-export time for single-row runs and 16% for long runs. Rust allocations per export loop fall from 146 to 112 and from 212 to 188, respectively. Standalone export and construction timings show no clear material change. Native DuckDB allocations were not measured. <details> <summary>Validation</summary> - After cleanup, `cargo nextest run -p vortex-duckdb -E 'test(exporter::run_end::)'`: 7 passed, 265 outside the filter. - Before cleanup, the full exporter filter passed all 80 tests. - `cargo clippy -p vortex-duckdb --all-targets --all-features -- -D warnings`: passed. - `cargo +nightly-2026-09-10 fmt -p vortex-duckdb`: passed. - Commit hook: workspace Rust and TOML formatting passed. - Pre-push hook: DCO checks and workspace Clippy with all targets, all features, and warnings denied passed. - Broader runtime suites and doctests were not run. </details> <details> <summary>Benchmark methodology and full results</summary> Baseline: `b367941e9d490b4a19ef9bdd6653f63d712c581f` with the same benchmark harness. Candidate: `702fd58dea3c0eeb214d590b3cc6d6b5b3c89c9b`. This comparison isolates the follow-up from the Arrow extension/property stack. The [measurement harness](https://github.com/vortex-data/vortex/blob/702fd58dea3c0eeb214d590b3cc6d6b5b3c89c9b/vortex-duckdb/benches/run_end_export.rs) and its Cargo target remain available in that measured commit. The permanent benchmark target was removed during review, along with duplicate test setup and the module split. The production algorithm is unchanged, but these measurements were not rerun after the file move. macOS 27.0.1, aarch64, rustc 1.99.1, LLVM 23.1.1, Cargo bench profile, and DuckDB 2.0.0. Build both revisions with `cargo bench -p vortex-duckdb --bench run_end_export --no-run`. Run their saved executables with `--bench --sample-count 300 --sample-size 20 --color never` in five pairs, alternating baseline/candidate order. The table shows the median of each executable's five run medians, plus the median of the five paired candidate/baseline ratios. These two aggregations need not agree exactly. Inputs are non-null i64 run values with run lengths 1 or 8192. Short-run sources have 16,383 runs, and long-run sources have eight. Chunked inputs repeat the same source four times. Every input is sliced from row 17 to 13 rows before its end, so exports span multiple batches and chunk boundaries fall inside output batches. Fixtures execute the slice to its encoded representation before exporter construction. Construction uses a fresh conversion cache per iteration. Batch export uses a warm cache and excludes exporter construction and data-chunk creation. `AllocProfiler<MiMalloc>` counts Rust allocations in the measured region, including its timing overhead. It does not count native DuckDB allocations. Neither `xctrace` nor `heaptrack` is available on this machine. Allocation figures below are allocation counts and requested bytes, not peak resident memory. | Phase | Input | Run length | Baseline (us) | Candidate (us) | Paired ratio | All five paired ratios | | --- | --- | ---: | ---: | ---: | ---: | --- | | batches | standalone | 1 | 14.02 | 13.51 | 0.961 | 0.961, 0.954, 0.968, 0.949, 1.028 | | batches | standalone | 8192 | 3.349 | 3.293 | 0.980 | 0.866, 1.007, 1.015, 0.926, 0.980 | | batches | shared chunks | 1 | 48.3 | 41.15 | 0.851 | 0.849, 0.851, 0.858, 0.852, 0.850 | | batches | shared chunks | 8192 | 19.74 | 16.57 | 0.839 | 0.839, 0.839, 0.833, 0.864, 0.832 | | construct | standalone | 1 | 0.8631 | 0.8464 | 0.981 | 0.951, 0.979, 0.984, 0.981, 1.016 | | construct | standalone | 8192 | 0.7922 | 0.7714 | 0.981 | 0.934, 0.982, 0.981, 0.966, 1.004 | | construct | shared chunks | 1 | 51.69 | 51.68 | 1.000 | 1.012, 0.999, 0.997, 1.000, 1.000 | | construct | shared chunks | 8192 | 12.9 | 12.88 | 0.998 | 1.002, 0.997, 0.998, 0.998, 0.998 | | Phase | Input | Run length | Baseline Rust allocations | Candidate Rust allocations | Baseline bytes | Candidate bytes | | --- | --- | ---: | ---: | ---: | ---: | ---: | | Batches | Standalone | 1 | 0 | 0 | 0 | 0 | | Batches | Standalone | 8192 | 0 | 0 | 0 | 0 | | Batches | Shared chunks | 1 | 146 | 112 | 193.1 KB | 130.5 KB | | Batches | Shared chunks | 8192 | 212 | 188 | 200.2 KB | 139.3 KB | | Construct | Standalone | 1 | 5 | 5 | 308 B | 308 B | | Construct | Standalone | 8192 | 5 | 5 | 308 B | 308 B | | Construct | Shared chunks | 1 | 32 | 32 | 265.8 KB | 265.8 KB | | Construct | Shared chunks | 8192 | 22 | 22 | 1.05 MB | 1.05 MB | The shared-chunk batch timing improved in every pair. Standalone controls and construction moved much less, and the standalone long-run ratios varied in direction. No compiler-output analysis was performed, so those small movements are not attributed to a particular code-generation change. These microbenchmarks do not establish an end-to-end query speedup. </details> --------- Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
## Summary QC `develop` is missing 13 upstream commits. Merge `vortex-data/vortex:develop` at `cb613ac1c0adc56cbe63dfc79e4bbc431ad0bc52` into QC `develop` at `b2c429ac2830b883cb37c1ff9262dc9e3cb7e483`. Merge this PR with a merge commit to preserve upstream ancestry for future syncs. ## Changes - Import typed Arrow extension plugins and round-trip metadata preservation, run-end export fixes, and DuckDB dictionary reuse across chunks ([vortex-data#11360](vortex-data#11360), [vortex-data#11375](vortex-data#11375), [vortex-data#11378](vortex-data#11378)). - Bring in improved string dictionary selection, spatial contains/within pruning, and safe PCO decoding ([vortex-data#9762](vortex-data#9762), [vortex-data#11372](vortex-data#11372), [vortex-data#11363](vortex-data#11363)). - Include CUDA Widen execution, bounded dictionary gathers, encoded-child host copies, and string dictionaries in the CUDA compression preset. - Reconcile the upstream Java I/O ports with their existing QC implementations, preserving read metrics, Kryo-safe Hadoop broadcasts, lifecycle handling, session defaults, and regression coverage. Correct the coalescing default's Javadoc to 1 MiB. ## Testing - Pinned `nightly-2026-09-10` Rust formatting for all affected crates, root/subproject `./gradlew spotlessApply`, and clang-format for the changed CUDA kernels passed. Reviewed and included the formatter diff. - `git diff --check` passed; no unresolved merge entries or conflict markers remain. The worktree is clean and both remote tips are parents of the signed-off merge commit. - All 13 imported commits have sign-off trailers and retain their original upstream metadata. - Tests, builds, lint, and benchmarks were not run locally, following repository guidance. --------- Signed-off-by: Daniel King <dan@spiraldb.com> Signed-off-by: Tanish Gudise <tgudise@andrew.cmu.edu> Signed-off-by: Alexander Droste <alexander.droste@protonmail.com> Signed-off-by: Christopher Wirt <cwirt@quantcast.com> Signed-off-by: Nemo Yu <zyu379@wisc.edu> Signed-off-by: Connor Tsui <connor.tsui20@gmail.com> Signed-off-by: Will Manning <will@willmanning.io> Signed-off-by: Robert Kruszewski <robert@spiraldb.com> Co-authored-by: Dan King <dan@spiraldb.com> Co-authored-by: Tanish Gudise <tgudise@andrew.cmu.edu> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Alexander Droste <alexander.droste@protonmail.com> Co-authored-by: Connor Tsui <connor.tsui20@gmail.com> Co-authored-by: Nemo Yu <83347615+HarukiMoriarty@users.noreply.github.com> Co-authored-by: Will Manning <will@spiraldb.com>
Summary
Builds on #11375.
Exporting a versioned Vortex UUID to Arrow and importing it back currently drops the declared version, even when every value is unchanged. Nested extensions can also lose metadata or fail to import when Arrow fields disagree with their storage. The full dtype, including nested metadata and nullability, must survive this round trip.
The raw import and export traits leave metadata transport to each plugin.
ArrowExtVTableputs that handling behind a typed registration adapter, so extension authors implement their native conversions while shared code preserves Vortex metadata.Changes
The session checks that inferred fields reconstruct the complete source dtype and that successful plugin imports return the dtype inferred from their field. These checks apply to raw and typed plugins. Explicit export targets can still request conversions.
UUID needs a separate Vortex payload because canonical
arrow.uuidmetadata must be empty. Keeping that representation lets other Arrow tools recognize UUID columns. A field without the Vortex payload imports with an unspecified version, so ordinary Arrow producers remain supported.Nested conversions need the full Arrow field because a
DataTypealone cannot carry extension identity. GeoArrow's required child names also belong to its layout contract. Metadata restoration uses inferred Vortex storage because it can differ from Arrow's physical storage.Regression coverage retains complete dtype and array comparisons while removing redundant parameter products and assertions. Variant remains excluded pending #11359, and equality derivation remains separate in #11115.
API Changes
Extension plugins implement
ArrowExtVTable, declaremetadata_policy(), and register throughArrowSession::register_extension(vtable). The typed import callbacks are associated functions. Raw plugins remain available, but inferred fields and imported arrays must satisfy the same dtype checks.Validation
nightly-2026-09-10.