fix: Permute the group bitmap with the data in sliceGroups - #219
Open
SamuelSchlesinger wants to merge 2 commits into
Open
fix: Permute the group bitmap with the data in sliceGroups#219SamuelSchlesinger wants to merge 2 commits into
SamuelSchlesinger wants to merge 2 commits into
Conversation
sliceGroups permutes a column's data into group order but slices the null bitmap at the group's offsets in the original row order, so a group's values and its validity bits come from different rows unless the frame already happens to be sorted by the key. Both cases use interleaved keys so valueIndices is not the identity permutation. Currently the first reports a=4.0 for a group summing to 10.0; the second mis-attributes nulls in both groups.
The data was backpermuted into group order while the bitmap was sliced at the group's offsets in the original row order, so values and validity bits came from different rows unless the frame was already sorted by the key. Permute the bitmap by the same indices, once, before slicing.
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
1 participant
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.
sliceGroupsbackpermutes the values into group order but slices the original, unpermuted bitmap, so nulls get attributed to whatever rows sit at those positions after sorting. A groupedsumMaybeover interleaved keys sums the wrong rows.Permute the bitmap with the same index vector before slicing. Two tests pin it (nulls in one group / nulls in every group).