scovich commented on code in PR #9295:
URL: https://github.com/apache/arrow-rs/pull/9295#discussion_r2748029144
##########
arrow-array/src/array/mod.rs:
##########
@@ -350,12 +413,19 @@ pub unsafe trait Array: std::fmt::Debug + Send + Sync {
/// A reference-counted reference to a generic `Array`
pub type ArrayRef = Arc<dyn Array>;
-/// Ergonomics: Allow use of an ArrayRef as an `&dyn Array`
+/// Ergonomics: Allow use of an ArrayRef as an `&dyn Array`.
+/// Use [`Array::as_array_ref_opt()`] to recover the original [`ArrayRef`] and
+/// [`downcast_array_ref()`] to downcast to an Arc of some concrete array type.
unsafe impl Array for ArrayRef {
fn as_any(&self) -> &dyn Any {
self.as_ref().as_any()
}
+ fn as_array_ref_opt(&self) -> Option<ArrayRef> {
+ // Recursively unwrap nested Arcs to find the deepest one
+ Some(innermost_array_ref(Cow::Borrowed(self)))
Review Comment:
IMO both `impl Array for Arc<dyn Array>` and `impl<T: Array> Array for &T`
were questionable (design?) decisions back whenever they showed up.
I expect it was intended to be ergonomic?
* The former allows passing `&ArrayRef` directly to functions that expect
`&dyn Array`, without needing to litter the code with explicit `as_ref()` calls.
* The latter allows code that expects `impl AsRef<dyn Array>` to work with
both owned and borrowed operands.
Part of me would love to just remove both of those, but that would
_definitely_ be a massive breaking change. As in, massive number of compilation
failures (even if easily fixed).
Even within arrow-rs itself, the whole `ArrayAccessor` and `ArrayIterator`
abstraction would break if we took away the blanket impl for `&T`, because
`ArrayAccessor::Item` is owned for some array types (primitive, boolean, etc)
and borrowed for others (GenericByteArray). I spent a few minutes trying to
untangle it, without success.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
