Skip to content

Commit 6505d2a

Browse files
authored
Revert "Seal Array trait (#9092)", mark Array as unsafe (#9234)
This reverts commit 721f373. # Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. --> - Discussed in #9184 - Closes #9184 # Rationale for this change <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> As discussed in Discussed in #9184, rather than sealing the `Array` trait, it's going to potentially be marked `unsafe` so that projects have time to find alternatives for their use-cases that are now satisfied by a custom `Array` implementation. # What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> Clean revert of #9092 # Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> Not applicable here # Are there any user-facing changes? Yes, users can no implement the `Array` trait in their codebases again. <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. If there are any breaking changes to public APIs, please call them out. -->
1 parent 40f69b4 commit 6505d2a

15 files changed

+42
-62
lines changed

arrow-array/src/array/boolean_array.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,9 +286,8 @@ impl BooleanArray {
286286
}
287287
}
288288

289-
impl super::private::Sealed for BooleanArray {}
290-
291-
impl Array for BooleanArray {
289+
/// SAFETY: Correctly implements the contract of Arrow Arrays
290+
unsafe impl Array for BooleanArray {
292291
fn as_any(&self) -> &dyn Any {
293292
self
294293
}

arrow-array/src/array/byte_array.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -462,9 +462,8 @@ impl<T: ByteArrayType> std::fmt::Debug for GenericByteArray<T> {
462462
}
463463
}
464464

465-
impl<T: ByteArrayType> super::private::Sealed for GenericByteArray<T> {}
466-
467-
impl<T: ByteArrayType> Array for GenericByteArray<T> {
465+
/// SAFETY: Correctly implements the contract of Arrow Arrays
466+
unsafe impl<T: ByteArrayType> Array for GenericByteArray<T> {
468467
fn as_any(&self) -> &dyn Any {
469468
self
470469
}

arrow-array/src/array/byte_view_array.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -820,9 +820,8 @@ impl<T: ByteViewType + ?Sized> Debug for GenericByteViewArray<T> {
820820
}
821821
}
822822

823-
impl<T: ByteViewType + ?Sized> super::private::Sealed for GenericByteViewArray<T> {}
824-
825-
impl<T: ByteViewType + ?Sized> Array for GenericByteViewArray<T> {
823+
/// SAFETY: Correctly implements the contract of Arrow Arrays
824+
unsafe impl<T: ByteViewType + ?Sized> Array for GenericByteViewArray<T> {
826825
fn as_any(&self) -> &dyn Any {
827826
self
828827
}

arrow-array/src/array/dictionary_array.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -692,9 +692,8 @@ impl<'a, T: ArrowDictionaryKeyType> FromIterator<&'a str> for DictionaryArray<T>
692692
}
693693
}
694694

695-
impl<T: ArrowDictionaryKeyType> super::private::Sealed for DictionaryArray<T> {}
696-
697-
impl<T: ArrowDictionaryKeyType> Array for DictionaryArray<T> {
695+
/// SAFETY: Correctly implements the contract of Arrow Arrays
696+
unsafe impl<T: ArrowDictionaryKeyType> Array for DictionaryArray<T> {
698697
fn as_any(&self) -> &dyn Any {
699698
self
700699
}
@@ -853,9 +852,7 @@ impl<'a, K: ArrowDictionaryKeyType, V> TypedDictionaryArray<'a, K, V> {
853852
}
854853
}
855854

856-
impl<K: ArrowDictionaryKeyType, V: Sync> super::private::Sealed for TypedDictionaryArray<'_, K, V> {}
857-
858-
impl<K: ArrowDictionaryKeyType, V: Sync> Array for TypedDictionaryArray<'_, K, V> {
855+
unsafe impl<K: ArrowDictionaryKeyType, V: Sync> Array for TypedDictionaryArray<'_, K, V> {
859856
fn as_any(&self) -> &dyn Any {
860857
self.dictionary
861858
}

arrow-array/src/array/fixed_size_binary_array.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -603,9 +603,8 @@ impl std::fmt::Debug for FixedSizeBinaryArray {
603603
}
604604
}
605605

606-
impl super::private::Sealed for FixedSizeBinaryArray {}
607-
608-
impl Array for FixedSizeBinaryArray {
606+
/// SAFETY: Correctly implements the contract of Arrow Arrays
607+
unsafe impl Array for FixedSizeBinaryArray {
609608
fn as_any(&self) -> &dyn Any {
610609
self
611610
}

arrow-array/src/array/fixed_size_list_array.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -463,9 +463,8 @@ impl From<FixedSizeListArray> for ArrayData {
463463
}
464464
}
465465

466-
impl super::private::Sealed for FixedSizeListArray {}
467-
468-
impl Array for FixedSizeListArray {
466+
/// SAFETY: Correctly implements the contract of Arrow Arrays
467+
unsafe impl Array for FixedSizeListArray {
469468
fn as_any(&self) -> &dyn Any {
470469
self
471470
}

arrow-array/src/array/list_array.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -527,9 +527,8 @@ impl<OffsetSize: OffsetSizeTrait> GenericListArray<OffsetSize> {
527527
}
528528
}
529529

530-
impl<OffsetSize: OffsetSizeTrait> super::private::Sealed for GenericListArray<OffsetSize> {}
531-
532-
impl<OffsetSize: OffsetSizeTrait> Array for GenericListArray<OffsetSize> {
530+
/// SAFETY: Correctly implements the contract of Arrow Arrays
531+
unsafe impl<OffsetSize: OffsetSizeTrait> Array for GenericListArray<OffsetSize> {
533532
fn as_any(&self) -> &dyn Any {
534533
self
535534
}

arrow-array/src/array/list_view_array.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -415,9 +415,8 @@ impl<OffsetSize: OffsetSizeTrait> ArrayAccessor for &GenericListViewArray<Offset
415415
}
416416
}
417417

418-
impl<OffsetSize: OffsetSizeTrait> super::private::Sealed for GenericListViewArray<OffsetSize> {}
419-
420-
impl<OffsetSize: OffsetSizeTrait> Array for GenericListViewArray<OffsetSize> {
418+
/// SAFETY: Correctly implements the contract of Arrow Arrays
419+
unsafe impl<OffsetSize: OffsetSizeTrait> Array for GenericListViewArray<OffsetSize> {
421420
fn as_any(&self) -> &dyn Any {
422421
self
423422
}

arrow-array/src/array/map_array.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -362,9 +362,8 @@ impl MapArray {
362362
}
363363
}
364364

365-
impl super::private::Sealed for MapArray {}
366-
367-
impl Array for MapArray {
365+
/// SAFETY: Correctly implements the contract of Arrow Arrays
366+
unsafe impl Array for MapArray {
368367
fn as_any(&self) -> &dyn Any {
369368
self
370369
}

arrow-array/src/array/mod.rs

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -78,18 +78,17 @@ pub use list_view_array::*;
7878

7979
use crate::iterator::ArrayIter;
8080

81-
mod private {
82-
/// Private marker trait to ensure [`super::Array`] can not be implemented outside this crate
83-
pub trait Sealed {}
84-
85-
impl<T: Sealed> Sealed for &T {}
86-
}
87-
8881
/// An array in the [arrow columnar format](https://arrow.apache.org/docs/format/Columnar.html)
8982
///
90-
/// This trait is sealed as it is not intended for custom array types, rather only
91-
/// those defined in this crate.
92-
pub trait Array: std::fmt::Debug + Send + Sync + private::Sealed {
83+
/// # Safety
84+
///
85+
/// Implementations of this trait must ensure that all methods implementations comply with
86+
/// the Arrow specification. No safety guards are placed and failing to comply with it can
87+
/// translate into panics or undefined behavior. For example, a value computed based on `len`
88+
/// may be used as a direct index into memory regions without checks.
89+
///
90+
/// Use it at your own risk knowing that this trait might be sealed in the future.
91+
pub unsafe trait Array: std::fmt::Debug + Send + Sync {
9392
/// Returns the array as [`Any`] so that it can be
9493
/// downcasted to a specific implementation.
9594
///
@@ -351,10 +350,8 @@ pub trait Array: std::fmt::Debug + Send + Sync + private::Sealed {
351350
/// A reference-counted reference to a generic `Array`
352351
pub type ArrayRef = Arc<dyn Array>;
353352

354-
impl private::Sealed for ArrayRef {}
355-
356353
/// Ergonomics: Allow use of an ArrayRef as an `&dyn Array`
357-
impl Array for ArrayRef {
354+
unsafe impl Array for ArrayRef {
358355
fn as_any(&self) -> &dyn Any {
359356
self.as_ref().as_any()
360357
}
@@ -433,7 +430,7 @@ impl Array for ArrayRef {
433430
}
434431
}
435432

436-
impl<T: Array> Array for &T {
433+
unsafe impl<T: Array> Array for &T {
437434
fn as_any(&self) -> &dyn Any {
438435
T::as_any(self)
439436
}

0 commit comments

Comments
 (0)