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

Add FilterFn trait + default implementation #458

Merged
merged 4 commits into from
Jul 16, 2024
Merged

Add FilterFn trait + default implementation #458

merged 4 commits into from
Jul 16, 2024

Conversation

a10y
Copy link
Contributor

@a10y a10y commented Jul 15, 2024

Implement filter and corresponding FilterFn array trait

See #439

@a10y a10y marked this pull request as ready for review July 15, 2024 16:44
@a10y a10y requested review from gatesn and joseph-isaacs July 15, 2024 16:44
///
/// We expect to change this soon once DataFusion is able to finish up some initial support, which
/// is tracked in https://github.com/apache/datafusion/issues/10918.
/// is tracked in <https://github.com/apache/datafusion/issues/10918>.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these were cleanups to make cargo doc -p vortex-array complete cleanly

@a10y a10y merged commit 9fa2f6e into develop Jul 16, 2024
2 checks passed
@a10y a10y deleted the aduffy/filter-fn branch July 16, 2024 08:49
@a10y a10y mentioned this pull request Jul 18, 2024

array.with_dyn(|a| {
if let Some(filter_fn) = a.filter() {
Ok(filter_fn.filter(array))
Copy link
Member

Choose a reason for hiding this comment

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

Should this be filter_fn.filter(predicate)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, yes

Copy link
Member

Choose a reason for hiding this comment

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

Ill fix it in a follow up.


pub trait FilterFn {
/// Filter an array by the provided predicate.
fn filter(&self, predicate: &Array) -> Array;
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about moving to this taking a &dyn BoolArrayTrait?

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