Repository navigation
fix(layout): rebase sliced list chunks before writing the list layout - #11082
Conversation
mhk197
left a comment
There was a problem hiding this comment.
Thanks for your contribution! I requested some changes wrt unnecessary tests, but the overall change is good.
The list writer stores each chunk's whole elements and shifts its offsets by the elements written before it, which assumes the offsets span exactly [0, elements.len()]. A sliced ListArray keeps its parent's elements, so its lists read back as the wrong elements. Reset each ListArray chunk's offsets first. ListView chunks, and so the default file writer, which canonicalizes before the list writer, were already trimmed. Signed-off-by: gitRasheed <[email protected]>
a9aebd7 to
d842053
Compare
Merging this PR will degrade performance by 11.35%2 warnings
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing |
Summary
ListLayoutStrategyassumes eachListArraychunk's offsets start at 0 and end atelements.len(). A slicedListArraykeeps its parent's wholeelements, so the writer stores offsets that point at the wrong elements, and the data reads back wrong with no error. For example, the lists[0, 1, 2], [], [3, 4, 5], [6, 7], [8, 9]written as three slices read back with row 1 as[3, 4, 5, 6, 7, 8, 9, 0, 1, 2]instead of[].The default file writer is not affected. It repartitions each list column into a canonical
ListViewfirst, and that conversion trims the elements. The bug hits writers that passListArraychunks to the list strategy directly, such asTableStrategy::with_list_layout().Changes
reset_offsetson eachListArraychunk before writing it.Tests
sliced_list_chunks_round_tripwrites slices of one list through the list writer and reads them back, with a null list and an empty list at the cuts.sliced_nested_list_chunks_round_tripdoes the same forlist<list<i32>>, where the inner lists arrive sliced too. Both fail without the change.Checked locally: the vortex-layout list tests,
clippy --all-targetsandfmt --checkfor vortex-layout and vortex-file.I used AI tools to help write and review this change.