diff --git a/pgx-tests/src/tests/array_tests.rs b/pgx-tests/src/tests/array_tests.rs index cc4e07306..07194d577 100644 --- a/pgx-tests/src/tests/array_tests.rs +++ b/pgx-tests/src/tests/array_tests.rs @@ -17,11 +17,11 @@ fn sum_array_i32(values: Array) -> i32 { let mut sum = 0_i32; for v in values { let v = v.unwrap_or(0); - let tmp = sum.overflowing_add(v); - if tmp.1 { + let (val, overflow) = sum.overflowing_add(v); + if overflow { panic!("attempt to add with overflow"); } else { - sum = tmp.0; + sum = val; } } sum @@ -99,6 +99,20 @@ fn return_zero_length_vec() -> Vec { Vec::new() } +#[pg_extern] +fn over_implicit_drop() -> Vec { + // Create an array of exactly Datum-sized numbers. + let mut vec: Vec = vec![1, 2, 3, 4, 5]; + let mut nulls = vec![false, false, true, false, false]; + // Verify we uphold the length contract. + assert_eq!(vec.len(), nulls.len()); + let len = vec.len(); + // Create an Array... + let _arr = unsafe { Array::<'_, i64>::over(vec.as_mut_ptr().cast(), nulls.as_mut_ptr(), len) }; + vec + // Implicit drop of _arr +} + #[cfg(any(test, feature = "pg_test"))] #[pgx::pg_schema] mod tests { @@ -240,4 +254,17 @@ mod tests { .expect("Failed to return json even though it's right there ^^"); assert_eq!(json.0, json! {{"values": [1, 2, 3, null, 4]}}); } + + #[pg_test] + fn test_array_over_direct() { + let vals = crate::tests::array_tests::over_implicit_drop(); + assert_eq!(vals, &[1, 2, 3, 4, 5]); + } + + #[pg_test] + fn test_array_over_spi() { + let vals: Vec = + Spi::get_one("SELECT over_implicit_drop();").expect("over machine broke"); + assert_eq!(vals, &[1, 2, 3, 4, 5]); + } } diff --git a/pgx/src/datum/array.rs b/pgx/src/datum/array.rs index 2ed2dd014..54c769814 100644 --- a/pgx/src/datum/array.rs +++ b/pgx/src/datum/array.rs @@ -7,17 +7,16 @@ All rights reserved. Use of this source code is governed by the MIT license that can be found in the LICENSE file. */ -use crate::{pg_sys, void_mut_ptr, FromDatum, IntoDatum, PgMemoryContexts}; +use crate::{pg_sys, FromDatum, IntoDatum, PgMemoryContexts}; use serde::Serializer; use std::marker::PhantomData; +use std::{mem, ptr, slice}; pub type VariadicArray<'a, T> = Array<'a, T>; pub struct Array<'a, T: FromDatum> { - ptr: *mut pg_sys::varlena, + _ptr: *mut pg_sys::varlena, array_type: *mut pg_sys::ArrayType, - elements: *mut pg_sys::Datum, - nulls: *mut bool, nelems: usize, elem_slice: &'a [pg_sys::Datum], null_slice: &'a [bool], @@ -50,40 +49,44 @@ impl<'a, T: FromDatum> Array<'a, T> { /// /// # Safety /// - /// This function is unsafe as it can't validate the provided pointer are valid or that - /// + /// This function requires that: + /// - `elements` is non-null + /// - `nulls` is non-null + /// - both `elements` and `nulls` point to a slice of equal-or-greater length than `nelems` pub unsafe fn over( elements: *mut pg_sys::Datum, nulls: *mut bool, nelems: usize, ) -> Array<'a, T> { Array:: { - ptr: std::ptr::null_mut(), - array_type: std::ptr::null_mut(), - elements, - nulls, + _ptr: ptr::null_mut(), + array_type: ptr::null_mut(), nelems, - elem_slice: std::slice::from_raw_parts(elements, nelems), - null_slice: std::slice::from_raw_parts(nulls, nelems), + elem_slice: slice::from_raw_parts(elements, nelems), + null_slice: slice::from_raw_parts(nulls, nelems), _marker: PhantomData, } } + /// # Safety + /// + /// This function requires that: + /// - `elements` is non-null + /// - `nulls` is non-null + /// - both `elements` and `nulls` point to a slice of equal-or-greater length than `nelems` unsafe fn from_pg( - ptr: *mut pg_sys::varlena, + _ptr: *mut pg_sys::varlena, array_type: *mut pg_sys::ArrayType, elements: *mut pg_sys::Datum, nulls: *mut bool, nelems: usize, ) -> Self { Array:: { - ptr, + _ptr, array_type, - elements, - nulls, nelems, - elem_slice: std::slice::from_raw_parts(elements, nelems), - null_slice: std::slice::from_raw_parts(nulls, nelems), + elem_slice: slice::from_raw_parts(elements, nelems), + null_slice: slice::from_raw_parts(nulls, nelems), _marker: PhantomData, } } @@ -94,15 +97,15 @@ impl<'a, T: FromDatum> Array<'a, T> { } let ptr = self.array_type; - std::mem::forget(self); + mem::forget(self); ptr } pub fn as_slice(&self) -> &[T] { - let sizeof_type = std::mem::size_of::(); - let sizeof_datums = std::mem::size_of_val(self.elem_slice); + let sizeof_type = mem::size_of::(); + let sizeof_datums = mem::size_of_val(self.elem_slice); unsafe { - std::slice::from_raw_parts( + slice::from_raw_parts( self.elem_slice.as_ptr() as *const T, sizeof_datums / sizeof_type, ) @@ -245,32 +248,6 @@ impl<'a, T: FromDatum> Iterator for ArrayIntoIterator<'a, T> { } } -impl<'a, T: FromDatum> Drop for Array<'a, T> { - fn drop(&mut self) { - if !self.elements.is_null() { - unsafe { - pg_sys::pfree(self.elements as void_mut_ptr); - } - } - - if !self.nulls.is_null() { - unsafe { - pg_sys::pfree(self.nulls as void_mut_ptr); - } - } - - if !self.array_type.is_null() && self.array_type as *mut pg_sys::varlena != self.ptr { - unsafe { - pg_sys::pfree(self.array_type as void_mut_ptr); - } - } - - // NB: we don't pfree(self.ptr) because we don't know if it's actually - // safe to do that. It'll be freed whenever Postgres deletes/resets its parent - // MemoryContext - } -} - impl<'a, T: FromDatum> FromDatum for Array<'a, T> { #[inline] unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool) -> Option> { @@ -294,10 +271,15 @@ impl<'a, T: FromDatum> FromDatum for Array<'a, T> { ); // outvals for deconstruct_array() - let mut elements = std::ptr::null_mut(); - let mut nulls = std::ptr::null_mut(); + let mut elements = ptr::null_mut(); + let mut nulls = ptr::null_mut(); let mut nelems = 0; + // FIXME: This way of getting array buffers causes problems for any Drop impl, + // and clashes with assumptions of Array being a "zero-copy", lifetime-bound array, + // some of which are implicitly embedded in other methods (e.g. Array::over). + // It also risks leaking memory, as deconstruct_array calls palloc. + // So either we don't use this, we use it more conditionally, or something. pg_sys::deconstruct_array( array, array_ref.elemtype,