Skip to content
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

feat: cache FilterMask iterators #1351

Merged
merged 11 commits into from
Nov 18, 2024
Merged

feat: cache FilterMask iterators #1351

merged 11 commits into from
Nov 18, 2024

Conversation

gatesn
Copy link
Contributor

@gatesn gatesn commented Nov 18, 2024

This PR uses a cute trick to pre-cache filter indices when a FilterMask is cloned. Unfortunately... we have to clone one every time we pass it into with_dyn!!

@lwwmanning lwwmanning changed the title FilterMask cached iterators feat: cache FilterMask iterators Nov 18, 2024
@lwwmanning lwwmanning enabled auto-merge (squash) November 18, 2024 16:37
@gatesn gatesn disabled auto-merge November 18, 2024 16:49
@@ -10,47 +10,60 @@ impl FilterFn for BoolArray {
let validity = self.validity().filter(&mask)?;

let buffer = match mask.iter()? {
FilterIter::Indices(iter) => {
filter_indices(self.boolean_buffer(), mask.true_count(), iter)
FilterIter::Indices(indices) => filter_indices_slice(&self.boolean_buffer(), indices),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calling filter_indices_slice for the indices branch looks... incorrect?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or, maybe correct but just confusing name

Copy link
Member

@lwwmanning lwwmanning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small naming nit, otherwise looks great

@gatesn gatesn enabled auto-merge (squash) November 18, 2024 17:55
@gatesn gatesn disabled auto-merge November 18, 2024 17:55
@gatesn gatesn merged commit 8f0ba91 into develop Nov 18, 2024
11 checks passed
@gatesn gatesn deleted the ngates/bit-packed-filter branch November 18, 2024 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants