I plan to merge this tomorrow, unless there are any additional comments

Thanks again @gabotechs

@scovich completely agree, and I think it makes sense that eventually this trait gets sealed. However, some crates are do willing to risk panics in order to benefit from the flexibility of trait Array, and this is a good intermediate step while these crates think or even contribute a better way of satisfying their needs.

@scovich completely agree, and I think it makes sense that eventually this trait gets sealed. However, some crates are do willing to risk panics in order to benefit from the flexibility of trait Array, and this is a good intermediate step while these crates think or even contribute a better way of satisfying their needs.

Understood. I was just commenting that the documentation in this PR, explaining why it's marked unsafe (strongly discouraged), doesn't give one of the biggest reasons. Maybe we should add that reason.

Or maybe there's enough truly unsafe code at risk to outweigh the "mere" panic risk of a cast gone bad?

doesn't give one of the biggest reasons. Maybe we should add that reason.

The closest thing it mentions is that "panics" can happen, it does not enumerate which places specifically. If you have any suggestions about more things to add to the safety comment they are more than welcome.

Skimming down the list of methods:

• Many code sites -- both internally and in user code -- use Array::data_type to drive casting decisions. Custom implementations of the Array trait risk causing panics in such code, unless Array::as_any actually returns the corresponding concrete type. See e.g. https://docs.rs/arrow/latest/arrow/array/macro.downcast_primitive_array.html
  • Corollary: Any type that attempts to impl Array as a "container" is utterly unusable as an actual array, because the original type could only be recovered if its Array::as_any returns a reference to the original type.
• Array::into_data and Array::to_data must produce a valid ArrayData (with a valid ArrayData::data_type), or risk causing panics and/or UB in downstream consumers such as make_array.
  • Corollary: make_array can never recover the original custom array type. It will instead recover whatever ArrayData::data_type indicated.
• Array::is_empty, Array::len, and Array::offset must be accurate, even if this array is the result of Array::slice
• Array::nulls must be accurate (any entry wrongly marked non-null has an undefined value)

Summing it all up -- any custom Array implementation must either be a complex analogue to dyn Any -- completely ignoring the normal Array API -- or must look and act exactly like a newtype wrapper around one of the built-in Array types (e.g. Arc<dyn Array> can safely impl Array).

• There seems to be a connection here with No way to downcast Arc<dyn Array> to Arc<T: Array> #8794, where Array::as_any as a replacement for Array: Any causes awkwardness in casting?

Problem is -- there's no way to tell which one you're dealing with, for an arbitrary &dyn Array. The only way to be sure you have a usable Array is to round trip it to ArrayData and back before attempting to work with it (assuming Array::into_data is implemented correctly).

If we really wanted to support the use case of dyn Array as a stand-in for dyn Any (since Array: !Any), then we'd have to define a new method, a "raw" version of Array::as_any, that can be downcast to recover the true type, and possibly also an analogue to Array::data_type that could guide the use of that raw casting. That way, the custom array-as-pointer could maintain the 1:1 correspondence between Array::data_type and Array::as_any, while still allowing to recover the original type. However, it seems like these are really just different use cases and we probably shouldn't try to conflate them in a single type.

Backport PR:

• [57_maintenance] Revert "Seal Array trait (#9092)", mark Array as unsafe (#9234) #9313

I also created a PR to try and summarize the discussion about DataType in the comments:

• Improve safety documentation of the Array trait #9314