Skip to content

Commit

Permalink
rename as_ptr -> storage_ptr and fix related bug
Browse files Browse the repository at this point in the history
  • Loading branch information
orlp committed Jan 15, 2024
1 parent 37e3baa commit ad784c7
Show file tree
Hide file tree
Showing 10 changed files with 19 additions and 60 deletions.
4 changes: 2 additions & 2 deletions crates/polars-arrow/src/array/binary/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ unsafe impl<O: Offset> ToFfi for BinaryArray<O> {
fn buffers(&self) -> Vec<Option<*const u8>> {
vec![
self.validity.as_ref().map(|x| x.as_ptr()),
Some(self.offsets.buffer().as_ptr().cast::<u8>()),
Some(self.values.as_ptr().cast::<u8>()),
Some(self.offsets.buffer().storage_ptr().cast::<u8>()),
Some(self.values.storage_ptr().cast::<u8>()),
]
}

Expand Down
4 changes: 2 additions & 2 deletions crates/polars-arrow/src/array/binview/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ unsafe impl<T: ViewType + ?Sized> ToFfi for BinaryViewArrayGeneric<T> {
fn buffers(&self) -> Vec<Option<*const u8>> {
let mut buffers = Vec::with_capacity(self.buffers.len() + 2);
buffers.push(self.validity.as_ref().map(|x| x.as_ptr()));
buffers.push(Some(self.views.as_ptr().cast::<u8>()));
buffers.extend(self.buffers.iter().map(|b| Some(b.as_ptr())));
buffers.push(Some(self.views.storage_ptr().cast::<u8>()));
buffers.extend(self.buffers.iter().map(|b| Some(b.storage_ptr())));
buffers
}

Expand Down
4 changes: 2 additions & 2 deletions crates/polars-arrow/src/array/binview/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ unsafe impl<T: ViewType + ?Sized> Sync for BinaryViewArrayGeneric<T> {}
fn buffers_into_raw<T>(buffers: &[Buffer<T>]) -> Arc<[(*const T, usize)]> {
buffers
.iter()
.map(|buf| (buf.as_ptr(), buf.len()))
.map(|buf| (buf.storage_ptr(), buf.len()))
.collect()
}

Expand Down Expand Up @@ -262,7 +262,7 @@ impl<T: ViewType + ?Sized> BinaryViewArrayGeneric<T> {
// data: 12 bytes

let bytes = if len <= 12 {
let ptr = self.views.as_ptr() as *const u8;
let ptr = self.views.storage_ptr() as *const u8;
std::slice::from_raw_parts(ptr.add(i * 16 + 4), len as usize)
} else {
let buffer_idx = (v >> 64) as u32;
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-arrow/src/array/fixed_size_binary/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ unsafe impl ToFfi for FixedSizeBinaryArray {
fn buffers(&self) -> Vec<Option<*const u8>> {
vec![
self.validity.as_ref().map(|x| x.as_ptr()),
Some(self.values.as_ptr().cast::<u8>()),
Some(self.values.storage_ptr().cast::<u8>()),
]
}

Expand Down
2 changes: 1 addition & 1 deletion crates/polars-arrow/src/array/list/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ unsafe impl<O: Offset> ToFfi for ListArray<O> {
fn buffers(&self) -> Vec<Option<*const u8>> {
vec![
self.validity.as_ref().map(|x| x.as_ptr()),
Some(self.offsets.buffer().as_ptr().cast::<u8>()),
Some(self.offsets.buffer().storage_ptr().cast::<u8>()),
]
}

Expand Down
2 changes: 1 addition & 1 deletion crates/polars-arrow/src/array/map/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ unsafe impl ToFfi for MapArray {
fn buffers(&self) -> Vec<Option<*const u8>> {
vec![
self.validity.as_ref().map(|x| x.as_ptr()),
Some(self.offsets.buffer().as_ptr().cast::<u8>()),
Some(self.offsets.buffer().storage_ptr().cast::<u8>()),
]
}

Expand Down
2 changes: 1 addition & 1 deletion crates/polars-arrow/src/array/primitive/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ unsafe impl<T: NativeType> ToFfi for PrimitiveArray<T> {
fn buffers(&self) -> Vec<Option<*const u8>> {
vec![
self.validity.as_ref().map(|x| x.as_ptr()),
Some(self.values.as_ptr().cast::<u8>()),
Some(self.values.storage_ptr().cast::<u8>()),
]
}

Expand Down
6 changes: 3 additions & 3 deletions crates/polars-arrow/src/array/union/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ unsafe impl ToFfi for UnionArray {
fn buffers(&self) -> Vec<Option<*const u8>> {
if let Some(offsets) = &self.offsets {
vec![
Some(self.types.as_ptr().cast::<u8>()),
Some(offsets.as_ptr().cast::<u8>()),
Some(self.types.storage_ptr().cast::<u8>()),
Some(offsets.storage_ptr().cast::<u8>()),
]
} else {
vec![Some(self.types.as_ptr().cast::<u8>())]
vec![Some(self.types.storage_ptr().cast::<u8>())]
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/polars-arrow/src/array/utf8/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ unsafe impl<O: Offset> ToFfi for Utf8Array<O> {
fn buffers(&self) -> Vec<Option<*const u8>> {
vec![
self.validity.as_ref().map(|x| x.as_ptr()),
Some(self.offsets.buffer().as_ptr().cast::<u8>()),
Some(self.values.as_ptr().cast::<u8>()),
Some(self.offsets.buffer().storage_ptr().cast::<u8>()),
Some(self.values.storage_ptr().cast::<u8>()),
]
}

Expand Down
49 changes: 4 additions & 45 deletions crates/polars-arrow/src/buffer/immutable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,13 @@ impl<T> Buffer<T> {
self.length = length;
}

/// Returns a pointer to the start of this buffer.
/// Returns a pointer to the start of the storage underlying this buffer.
#[inline]
pub(crate) fn as_ptr(&self) -> *const T {
self.ptr
pub(crate) fn storage_ptr(&self) -> *const T {
self.storage.as_ptr()
}

/// Returns the offset of this buffer.
/// Returns the start offset of this buffer within the underlying storage.
#[inline]
pub fn offset(&self) -> usize {
unsafe {
Expand Down Expand Up @@ -220,21 +220,6 @@ impl<T> Buffer<T> {
}
}

/// Returns a mutable reference to its underlying `Vec`, if possible.
/// Note that only `[self.offset(), self.offset() + self.len()[` in this vector is visible
/// by this buffer.
///
/// This operation returns [`Some`] iff this [`Buffer`]:
/// * has not been cloned (i.e. [`Arc`]`::get_mut` yields [`Some`])
/// * has not been imported from the C data interface (FFI)
/// # Safety
/// The caller must ensure that the vector in the mutable reference keeps a length of at least `self.offset() + self.len() - 1`.
#[inline]
pub unsafe fn get_mut(&mut self) -> Option<&mut Vec<T>> {
// Arc::get_mut(&mut self.storage).and_then(|b| b.get_vec())
unimplemented!()
}

/// Returns a mutable reference to its slice, if possible.
///
/// This operation returns [`Some`] iff this [`Buffer`]:
Expand All @@ -257,32 +242,6 @@ impl<T> Buffer<T> {
pub fn shared_count_weak(&self) -> usize {
Arc::weak_count(&self.storage)
}

/// Returns its internal representation
#[must_use]
pub fn into_inner(self) -> (Arc<Bytes<T>>, usize, usize) {
// let Self {
// data,
// offset,
// length,
// } = self;
// (data, offset, length)
unimplemented!();
}

/// Creates a `[Bitmap]` from its internal representation.
/// This is the inverted from `[Bitmap::into_inner]`
///
/// # Safety
/// Callers must ensure all invariants of this struct are upheld.
pub unsafe fn from_inner_unchecked(_data: Arc<Bytes<T>>, _offset: usize, _length: usize) -> Self {
// Self {
// data,
// offset,
// length,
// }
unimplemented!();
}
}

impl<T: Zero + Copy> Buffer<T> {
Expand Down

0 comments on commit ad784c7

Please sign in to comment.