[Rust] Move filter and limit code from DataFusion into compute module #21145

Activity

1. asfimport commented on Feb 18, 2019

   

   asfimport

   on Feb 18, 2019

   Author

   More actions

   Nicolas Trinquier / @ntrinquier:
   @andygrove did you mean to move the functions as they are? They both do some kind of filtering on the data, what do you think of re-implementing them as a more generic function?

   Some along those lines:

   fn filter(a: &Array, predicate: Fn(usize) -> bool) -> Result<ArrayRef> {
   ...
   
     for i in 0..b.len() {
       if predicate(i) {
         builder.append_value(a.value(i))?
       }
     }
   ...
   }

    

   Predicates would look like this:

   let limit_predicate = |index| { index < limit_value }
   let filter_predicate = |index| { filter_bools.value(index) }

   I do not know a/ if this pattern is very rustacean, and b/ if the abstraction is worth it (i.e. in the case of limit we would still allocate a buffer for the full size and iterate through all the elements whereas we could save space and return early).

    
2. asfimport commented on Feb 18, 2019

   

   asfimport

   on Feb 18, 2019

   Author

   More actions

   Nicolas Trinquier / @ntrinquier:
   Maybe it's better to have 2 functions and define 2 macros, for limit and filter.
3. asfimport commented on Feb 19, 2019

   

   asfimport

   on Feb 19, 2019

   Author

   More actions

   Neville Dipale / @nevi-me:
   Hi @ntrinquier, for filter, you might consider 
   fn filter(a: &Array, predicate: &BooleanArray)
   so the predicate evaluation is evaluated once, and a boolean array passed to filter each array.
4. asfimport commented on Feb 19, 2019

   

   asfimport

   on Feb 19, 2019

   Author

   More actions

   Nicolas Trinquier / @ntrinquier:
   filter has already the signature you suggest [~nevi_me].

   However for limit it is only a usize, not an array of bools, so I initially suggested to have one function to do both filter and limit, with the signature:
   filter(a: &Array, predicate: Fn(usize) -> bool) -> Result
   but on second though I am not sure this is worth it because of the performance implications. We might be better off using macros. What do you think?
5. asfimport commented on Feb 24, 2019

   

   asfimport

   on Feb 24, 2019

   Author

   More actions

   Andy Grove / @andygrove:
   Issue resolved by pull request 3741
   #3741
6. asfimport

   closed this as completedon Feb 24, 2019
7. asfimport

   added this to the 0.13.0 milestone on Jan 11, 2023