-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Minor: use upstream RowSelection code from arrow intersect_row_selection
#4340
Conversation
Cargo.toml
Outdated
|
||
|
||
[patch.crates-io] | ||
arrow = { git = "https://github.com/alamb/arrow-rs.git", rev="7fd330b" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Waiting on arrow 28.0.0 release
intersect_row_selection
d2db743
to
1b73c51
Compare
@@ -123,7 +122,7 @@ pub(crate) fn build_page_filter( | |||
if let (Some(file_offset_indexes), Some(file_page_indexes)) = | |||
(file_offset_indexes, file_page_indexes) | |||
{ | |||
let mut row_selections = VecDeque::with_capacity(page_index_predicates.len()); | |||
let mut row_selections = Vec::with_capacity(page_index_predicates.len()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since row_selections
are always appended to (never pop_front, for example) I think Vec is just fine and using VecDequeue just makes the code more confusing
1b73c51
to
5634163
Compare
/// set `need_combine` true will combine result: Select(2) + Select(1) + Skip(2) -> Select(3) + Skip(2) | ||
/// | ||
/// Move to arrow-rs: https://github.com/apache/arrow-rs/issues/3003 | ||
pub(crate) fn intersect_row_selection( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
upstreamed in apache/arrow-rs#3173
Benchmark runs are scheduled for baseline = 27dc295 and contender = 8db99d2. 8db99d2 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Draft as it is waiting onRowSelection::iter()
,Into<Vec<RowSelector>>
and example arrow-rs#3173Which issue does this PR close?
N/A
Rationale for this change
This code was upstreamed in apache/arrow-rs#3003
What changes are included in this PR?
Remove Datafusion copy of
intersect_row_selection
which has been upstreamedAre these changes tested?
Yes by existing tests
Are there any user-facing changes?
No