Add ArrayAccessor trait, remove duplication in array iterators (#1948) #2133
Conversation
There was a problem hiding this comment.
I think this is very cool -- nice work @tustvold
cc @viirya and @HaoYang670 and @jhorstmann
I wonder if we should run benchmarks to verify this does not introduce a performance regression
| } | ||
|
|
||
| /// A generic trait for accessing the values of an [`Array`] | ||
| pub trait ArrayAccessor: Array { |
| // Note: This implementation is based on std's [Vec]s' [IntoIter]. | ||
| #[derive(Debug)] | ||
| pub struct PrimitiveIter<'a, T: ArrowPrimitiveType> { | ||
| array: &'a PrimitiveArray<T>, | ||
| pub struct ArrayIter<T: ArrayAccessor> { |
alamb
changed the title
tustvold
force-pushed
the
add-array-accessor
branch
from
8948a9d
to
b06996b
Compare
| /// an iterator that returns Some(T) or None, that can be used on any PrimitiveArray | ||
| pub type PrimitiveIter<'a, T> = ArrayIter<&'a PrimitiveArray<T>>; | ||
| pub type BooleanIter<'a> = ArrayIter<&'a BooleanArray>; | ||
| pub type GenericStringIter<'a, T> = ArrayIter<&'a GenericStringArray<T>>; | ||
| pub type GenericBinaryIter<'a, T> = ArrayIter<&'a GenericBinaryArray<T>>; | ||
| pub type GenericListArrayIter<'a, O> = ArrayIter<&'a GenericListArray<O>>; |
|
Out benchmark coverage of these iterators does not appear to be excellent, but I'm not seeing anything to suggest this represents a regression |
Codecov Report
@@ Coverage Diff @@
## master #2133 +/- ##
==========================================
- Coverage 83.71% 83.65% -0.06%
==========================================
Files 225 225
Lines 59567 59606 +39
==========================================
- Hits 49865 49862 -3
- Misses 9702 9744 +42
Help us with your feedback. Take ten seconds to tell us how you rate us. |
|
Benchmark runs are scheduled for baseline = 866f1a1 and contender = 91b2d7e. 91b2d7e is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Part of #1948
Rationale for this change
This makes it possible to write code that is generic over more types, which is particularly useful for #1764 where otherwise
GenericStringArrayandGenericBinaryArrayhave to be handled separately.What changes are included in this PR?
Adds an
ArrayAccessortrait and uses this to reduce duplication within the iteratorsAre there any user-facing changes?
No