You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/06/26 17:36:54 UTC

[GitHub] [arrow-rs] jhorstmann opened a new issue, #1948: Reduce macro usage by abstracting over arrays that have a value or value_unchecked method

jhorstmann opened a new issue, #1948:
URL: https://github.com/apache/arrow-rs/issues/1948

   **Is your feature request related to a problem or challenge? Please describe what you are trying to do.**
   
   Many Array types have a `value` and `value_unchecked` method. This method is not part of the `Array` trait because the return type would need to be a generic parameter, which would make the `Array` trait no longer object safe.
   
   The absence of such a method makes it more difficult to abstract over different array types. For example the [iterators have a lot of code duplication](https://github.com/apache/arrow-rs/blob/9f7b6004d365b0c0bac8e30170b49bdd66cc7df0/arrow/src/array/iterator.rs#L135) or the [comparison functions use macros as a workaround](https://github.com/apache/arrow-rs/blob/9f7b6004d365b0c0bac8e30170b49bdd66cc7df0/arrow/src/compute/kernels/comparison.rs#L60).
   
   **Describe the solution you'd like**
   
   I propose to add another trait (maybe named `ArrayAccessor`) that would unify the types:
   
   ```rust
   trait ArrayAccessor: Array {
       type Item;
       fn value(&self, index: usize) -> Self::Item;
       unsafe fn value_unchecked(&self, index: usize) -> Self::Item;
   }
   ```
   
   This could be implemented by `PrimitiveArray`, `BooleanArray`, `StringArray`, `BinaryArray, `ListArray` and maybe more.
   
   An interesting case is `DictionaryArray` where we might want two different accessors, one for keys and another that resolves keys to their corresponding values (haven't checked whether this would be possible to implement in a generic way).
   
   Iterators and comparison kernels could then be implement in a generic way using this trait, reducing the amount of macros in the code base.
   
   **Describe alternatives you've considered**
   
   We could keep using macros.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] tustvold closed issue #1948: Reduce macro usage by abstracting over arrays that have a value or value_unchecked method

Posted by GitBox <gi...@apache.org>.
tustvold closed issue #1948: Reduce macro usage by abstracting over arrays that have a value or value_unchecked method
URL: https://github.com/apache/arrow-rs/issues/1948


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] tustvold commented on issue #1948: Reduce macro usage by abstracting over arrays that have a value or value_unchecked method

Posted by GitBox <gi...@apache.org>.
tustvold commented on issue #1948:
URL: https://github.com/apache/arrow-rs/issues/1948#issuecomment-1229255111

   The ArrayAccessor trait has been added and is slowly being integrated, I think this issue can now be closed as it isn't tracking anything specific


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] martin-g commented on issue #1948: Reduce macro usage by abstracting over arrays that have a value or value_unchecked method

Posted by GitBox <gi...@apache.org>.
martin-g commented on issue #1948:
URL: https://github.com/apache/arrow-rs/issues/1948#issuecomment-1167394346

   > ```rust
   >  fn value(&self, index: usize) -> Self::Item;
   
   Shouldn't this return `Option<Self::Item>` ?
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] tustvold commented on issue #1948: Reduce macro usage by abstracting over arrays that have a value or value_unchecked method

Posted by GitBox <gi...@apache.org>.
tustvold commented on issue #1948:
URL: https://github.com/apache/arrow-rs/issues/1948#issuecomment-1171264848

   > as long as it is only used by methods that have an explicit where Self: Sized bound.
   
   Unfortunately this would require you to specify the associated type as part of constructing the trait object, i.e. `Box<dyn Array<Item = Foo>>`.
   
   That being said, having a separate `ArrayAccessor` trait makes a lot of sense to me


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] jhorstmann commented on issue #1948: Reduce macro usage by abstracting over arrays that have a value or value_unchecked method

Posted by GitBox <gi...@apache.org>.
jhorstmann commented on issue #1948:
URL: https://github.com/apache/arrow-rs/issues/1948#issuecomment-1167372005

   After reading https://doc.rust-lang.org/reference/items/traits.html#object-safety again this could also work as an associated type on `Array` itself, as long as it is only used by methods that have an explicit `where Self: Sized` bound.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] HaoYang670 commented on issue #1948: Reduce macro usage by abstracting over arrays that have a value or value_unchecked method

Posted by GitBox <gi...@apache.org>.
HaoYang670 commented on issue #1948:
URL: https://github.com/apache/arrow-rs/issues/1948#issuecomment-1167297153

   Could we add an associated type for `trait Array` directly?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org