Replace RawPtrBox with Safe Abstraction

[tustvold]

Is your feature request related to a problem or challenge? Please describe what you are trying to do.

Currently the Array implementations make use of RawPtrBox to reference data on their attached ArrayData. Not only is this construction deeply unsafe, but requires further unsafe pointer manipulation to use.

Describe the solution you'd like

I would like for arrays to store strongly typed slices, instead of strongly typed pointers. This necessarily implies a self-referential struct, fortunately ouroboros provides a safe interface that handles this complexity for you. The hardest part is actually spelling the crates' name...

#[self_referencing]
pub struct PrimitiveArray<T: ArrowPrimitiveType> {
    data: ArrayData,
    #[borrows(data)]
    values: &'this [T::Native]
}

This would eliminate a lot of unsafe, without requiring any breaking changes.

Describe alternatives you've considered

We could not do this, this might just be a way to reduce the amount of unsafe in arrow-rs without requiring any breaking changes.

We could also abstract the self-referential shenanigans in a typed buffer abstraction, something like

#[self_referencing]
struct TypedBuffer<T: ArrowNativeType> {
  inner: Buffer,
  #[borrows(inner)]
  values: &'this [T]
}

Possibly even dropping ouroborous in favor of a little bit of custom unsafe.

Related Context

Somewhat related to #1799 which introduces stronger typing within ArrayData itself.

We used ouroboros for a while in IOx to handle flatbuffers, and it did the job. We have since removed flatbuffers in favor of a columnar protobuf representation, but that was more a reflection on flatbuffers than ouroboros.

Thoughts @alamb @jhorstmann @jorgecarleitao @HaoYang670 ?

[HaoYang670]

The ouroboros is a personal crate, I am not sure whether it is safe to depend on it.

[tustvold]

Safe in what sense of the word? 😅

[HaoYang670]

Safe in what sense of the word? 😅

If there is a bug in ouroboros, then how to fix it?
What if the crate is out of maintenance?

[tustvold]

Definitely valid concerns, I think they need to be balanced against the likelihood of a bug in the current custom self-referential struct logic, vs an upstream that's had more eyes on it.

TBC I'm not totally sold on this proposal, I happen to think the current logic is probably fine, but opinions on the dangers of unsafe vary. Let's see what other people think 😅

[HaoYang670]

Sorry, I think I lack of the knowledge about why we have to use raw pointers here?

[tustvold]

The TLDR is

• ArrayData stores untyped ref-counted buffers of bytes inline with the arrow specification
• We want to provide a safe, strongly typed array interface to users (Array)
• We want to perform alignment, etc... checks once on creation
• We therefore construct typed RawPtrBox that reference the bytes owned by the Buffers stored in ArrayData on construction of an Array

Whilst I write this I realise we could encapsulate the self-referential shenanigans in a typed buffer abstraction. I'll update the issue 👍

[jhorstmann]

Interesting, haven't seen or tried that crate before, but seems to solve the exact problem that our Array types have where we have data owned by ArrayData but to access it without indirections using a pointer or slice. The crate looks healthy, several contributors and releases. The macro might slow down compilation a bit, but I think that's a worthwhile tradeoff.

RawPtrBox also implements Send + Sync, I think that should also work automatically for slices.

[alamb]

I like the idea of a typed buffer 👍 that lives in arrow and doesn't rely on some other crate

I agree with @HaoYang670 that hiding the unsafely behind someone else's library may just be fooling ourselves into thinking arrow is safer than it is.

[tustvold]

I bashed out what this might look like in #1820 PTAL