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 Array::as_any_arc
#2902
add Array::as_any_arc
#2902
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,7 @@ use arrow_buffer::ArrowNativeType; | |
use arrow_data::ArrayData; | ||
use arrow_schema::{ArrowError, DataType}; | ||
use std::any::Any; | ||
use std::sync::Arc; | ||
|
||
/// | ||
/// A dictionary array where each element is a single value indexed by an integer key. | ||
|
@@ -520,6 +521,10 @@ impl<T: ArrowPrimitiveType> Array for DictionaryArray<T> { | |
self | ||
} | ||
|
||
fn as_any_arc(self: Arc<Self>) -> Option<Arc<dyn Any + Send + Sync + 'static>> { | ||
Some(self) | ||
} | ||
|
||
fn data(&self) -> &ArrayData { | ||
&self.data | ||
} | ||
|
@@ -597,6 +602,10 @@ impl<'a, K: ArrowPrimitiveType, V: Sync> Array for TypedDictionaryArray<'a, K, V | |
self.dictionary | ||
} | ||
|
||
fn as_any_arc(self: Arc<Self>) -> Option<Arc<dyn Any + Send + Sync + 'static>> { | ||
None | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm... Perhaps we should remove the requirement for ArrayAccessor to impl Array 🤔 |
||
} | ||
|
||
fn data(&self) -> &ArrayData { | ||
&self.dictionary.data | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -88,6 +88,31 @@ pub trait Array: std::fmt::Debug + Send + Sync { | |
/// ``` | ||
fn as_any(&self) -> &dyn Any; | ||
|
||
/// Returns the array as [`Any`](std::any::Any) wrapped into an [`Arc`] so that it can be | ||
/// downcasted to a specific implementation without the need for an unsized reference. This is helpful if you want | ||
/// to return a concrete array type. | ||
/// | ||
/// Note that this conversion may not be possible for all types due to the `'static` constraint. | ||
/// | ||
/// # Example: | ||
/// | ||
/// ``` | ||
/// # use std::sync::Arc; | ||
/// # use arrow_array::{Int32Array, RecordBatch}; | ||
/// # use arrow_schema::{Schema, Field, DataType, ArrowError}; | ||
/// | ||
/// let id = Int32Array::from(vec![1, 2, 3, 4, 5]); | ||
/// let batch = RecordBatch::try_new( | ||
/// Arc::new(Schema::new(vec![Field::new("id", DataType::Int32, false)])), | ||
/// vec![Arc::new(id)] | ||
/// ).unwrap(); | ||
/// | ||
/// let int32array = Arc::downcast::<Int32Array>( | ||
/// Arc::clone(batch.column(0)).as_any_arc().expect("Failed to create static Arc") | ||
/// ).expect("Failed to downcast"); | ||
Comment on lines
+110
to
+112
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should provide a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding a helpers sounds great. Regarding privacy: do we consider There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the nature of the way that the downcasting works means that Array is effectively sealed as there would be no use to implementing it |
||
/// ``` | ||
fn as_any_arc(self: Arc<Self>) -> Option<Arc<dyn Any + Send + Sync + 'static>>; | ||
|
||
/// Returns a reference to the underlying data of this array. | ||
fn data(&self) -> &ArrayData; | ||
|
||
|
@@ -258,6 +283,10 @@ impl Array for ArrayRef { | |
self.as_ref().as_any() | ||
} | ||
|
||
fn as_any_arc(self: Arc<Self>) -> Option<Arc<dyn Any + Send + Sync + 'static>> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was really confused how this was working, and then realised this is actually implementing Perhaps this should return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No it returns Alternatively we could clone the inner
Ditto, hence my suggestion to hide it away in a private trait so we can revisit it later perhaps |
||
Some(self) | ||
} | ||
|
||
fn data(&self) -> &ArrayData { | ||
self.as_ref().data() | ||
} | ||
|
@@ -316,6 +345,10 @@ impl<'a, T: Array> Array for &'a T { | |
T::as_any(self) | ||
} | ||
|
||
fn as_any_arc(self: Arc<Self>) -> Option<Arc<dyn Any + Send + Sync + 'static>> { | ||
None | ||
} | ||
|
||
fn data(&self) -> &ArrayData { | ||
T::data(self) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would still work with
Option<Arc<dyn Any>>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get your question. Is that that you're wondering about
Send + Sync + 'static
? They are required forArc::downcast
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh the Send + Sync is required, but I think you can drop the
'static
as it is implied byAny
(and lifetime erasure for trait objects)