Repository navigation
fix(layout): normalize list-view children before serialization - #109
Merged
Merged
Conversation
Signed-off-by: Christopher Wirt <cwirt@quantcast.com>
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
Fix the shared Linux musl builder failure on
developand PRs #103, #105, #106, #107, and #108 after the upstream lazy-constructor update.Changes
Normalize lists rebuilt from list views through the same
reset_offsetspath 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 and every completed failing PR builder inspected (#105–#108).
Failed Jobs
Rust tests (linux-musl): test failure inlayouts::list::reader::tests::sliced_list_chunks_round_trip::case_4_list_view_chunks.Relevant Log Output
Correlation With PR Changes
The upstream merge makes array slicing lazy. The list-view branch of
canonicalize_to_list_partsforwarded 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
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.git diff --check: passed.Local validation ran on macOS; the fresh Linux musl builders will verify the runner-specific job.