Repository navigation
fix(spark): infer schemas from one footer by default - #105
Merged
Merged
Conversation
Signed-off-by: Christopher Wirt <[email protected]>
Signed-off-by: Christopher Wirt <[email protected]> (cherry picked from commit bfbc5bb) Signed-off-by: Christopher Wirt <[email protected]>
ChristopherWirt
added a commit
that referenced
this pull request
Nov 10, 2026
## Summary Fix the shared Linux musl builder failure on `develop` and PRs #103, #105, #106, #107, and #108 after the upstream lazy-constructor update. ## Changes Normalize lists rebuilt from list views through the same `reset_offsets` path as ordinary lists before forwarding their children to layout writers. Rebuilding a sliced list view can now leave a lazy elements slice; a flat child writer cannot serialize that expression. This preserves the existing referenced-element bounds and resolves the slice without forcing unrelated encodings into canonical arrays. Keep the existing nullable sliced-list-view regression and add the non-nullable case. ## CI Failure Analysis ### Status Pre-existing base-branch regression, independent of the individual PR changes. The same failure appears on [develop run 42649358621](https://github.com/quantcast/vortex/actions/runs/42649358621) and every completed failing PR builder inspected (#105–#108). ### Failed Jobs - `Rust tests (linux-musl)`: test failure in `layouts::list::reader::tests::sliced_list_chunks_round_trip::case_4_list_view_chunks`. ### Relevant Log Output ```text Error: Array vortex.slice is not registered for serialization ``` ### Correlation With PR Changes The upstream merge makes array slicing lazy. The list-view branch of `canonicalize_to_list_parts` forwarded a rebuilt list's lazy elements directly, while the ordinary-list branch already normalized them. The Spark PRs do not change this writer; #107 changes separate zone-pruning code. ### Recommended Next Step Merge this shared fix into `develop`. The same patch is being applied to the affected open PR branches so their new builders include the fix immediately. ## Validation - Reproduced the exact failing test on unmodified current `develop` (exit 100, same serialization error). - `cargo nextest run -p vortex-layout -E 'test(layouts::list::)'`: all 161 tests passed after the fix, including the original failing case and new non-nullable case. - `cargo clippy -p vortex-layout --all-targets --all-features -- -D warnings`: passed. - Pinned nightly Rust formatting and `git diff --check`: passed. Local validation ran on macOS; the fresh Linux musl builders will verify the runner-specific job. Signed-off-by: Christopher Wirt <[email protected]>
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.
Schema inference currently reads every Vortex footer on the driver before starting work. Default
vortex.mergeSchematofalse, so both V1 and V2 infer from one file. Datasets with evolving columns can explicitly opt into merging withvortex.mergeSchema=true; an explicit schema still skips inference. Update the Spark guide and API documentation to describe the new default and merging cost.Preserve explicit merge coverage and add V1/V2 regression cases for omitted and false options, plus case-insensitive opt-in coverage.
Validation:
VortexSchemaMergeTest,VortexDataSourceInferSchemaTest,VortexOptionsTest,VortexTableOptionsTest, andVortexSessionOptionsTest, using the baseline JNI library.git diff --checkpassed.Fixes #100