Parquet: add union method to RowSelection
#6308
Merged
Conversation
This file contains 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
Complements the existing RowSelection::intersection method. Useful for Or-ing row selections together, in contrast to intersection's use when AND-ing selections
sdd
changed the title
union method to RowSelection
crepererum
reviewed
Aug 26, 2024
| /// | ||
| /// returned: NYYYYYNNYYNYN | ||
| pub fn union(&self, other: &Self) -> Self { | ||
| union_row_selections(&self.selectors, &other.selectors) |
There was a problem hiding this comment.
why do we need an indirection here? as far as I can tell, the called method isn't used anywhere else.
There was a problem hiding this comment.
I based it on the intersection method, which has this structure. I'm not familiar with this code base and the design decisions so I just went with what was there. I can refactor if required.
crepererum
approved these changes
Aug 27, 2024
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.
Complements the existing
RowSelection::intersectionmethod. Useful for OR-ing row selections together, complementingthe use of
RowSelection::intersectionfor AND-ing row selectionsWhich issue does this PR close?
Closes #6307.
Are there any user-facing changes?
Yes. Doc comments for
RowSelection::unionhave been added accordingly as part of this PR.