Skip to content

Commit c5281a6

Browse files
committed
fix: detoast borrowed FlatArray arguments
1 parent 58147f1 commit c5281a6

5 files changed

Lines changed: 237 additions & 60 deletions

File tree

pgrx-sql-entity-graph/src/pg_extern/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,8 +468,8 @@ impl PgExtern {
468468
let call_flow = <#ret_ty as ::pgrx::callconv::RetAbi>::check_and_prepare(fcinfo);
469469
let result = match call_flow {
470470
::pgrx::callconv::CallCx::WrappedFn(mcx) => {
471+
let mut #args_ident = unsafe { fcinfo.args_in(mcx) };
471472
let mut mcx = ::pgrx::PgMemoryContexts::For(mcx);
472-
let #args_ident = &mut fcinfo.args();
473473
let call_result = mcx.switch_to(|_| {
474474
#(#arg_fetches)*
475475
#func_name( #(#arg_pats),* )

pgrx-unit-tests/src/tests/array_borrowed.rs

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,11 @@ fn borrow_get_arr_nelems(arr: &FlatArray<'_, i32>) -> libc::c_int {
9595
arr.nelems() as _
9696
}
9797

98+
#[pg_extern]
99+
fn borrow_iter_array<'a>(arr: &'a FlatArray<'a, i32>) -> SetOfIterator<'a, i32> {
100+
SetOfIterator::new(arr.iter_non_null().copied())
101+
}
102+
98103
#[pg_extern]
99104
fn borrow_get_arr_data_ptr_nth_elem(arr: &FlatArray<'_, i32>, elem: i32) -> Option<i32> {
100105
arr.get(elem as usize).unwrap().into_option().copied()
@@ -181,7 +186,7 @@ mod tests {
181186
use pgrx::datum::DatumWithOid;
182187
use pgrx::memcx;
183188
use pgrx::prelude::*;
184-
use pgrx::{IntoDatum, Json};
189+
use pgrx::{IntoDatum, Json, PgMemoryContexts, direct_pg_extern_function_call};
185190
use serde_json::json;
186191

187192
#[pg_test]
@@ -330,6 +335,67 @@ mod tests {
330335
assert_eq!(len, Ok(Some(5)));
331336
}
332337

338+
#[pg_test]
339+
fn borrow_test_toasted_flat_array() -> Result<(), pgrx::spi::Error> {
340+
Spi::run("CREATE TEMP TABLE flat_array_toast_test (arr integer[])")?;
341+
Spi::run("ALTER TABLE flat_array_toast_test ALTER COLUMN arr SET STORAGE EXTERNAL")?;
342+
Spi::run(
343+
"INSERT INTO flat_array_toast_test \
344+
SELECT array_agg(i) FROM generate_series(1, 2500) i",
345+
)?;
346+
347+
let result = Spi::get_two::<i32, i32>(
348+
"SELECT borrow_get_arr_nelems(arr), borrow_sum_array(arr) \
349+
FROM flat_array_toast_test",
350+
);
351+
assert_eq!(result, Ok((Some(2500), Some(3_126_250))));
352+
353+
let iterated = Spi::get_two::<i64, i64>(
354+
"SELECT count(*), sum(value) \
355+
FROM flat_array_toast_test, LATERAL borrow_iter_array(arr) value",
356+
);
357+
assert_eq!(iterated, Ok((Some(2500), Some(3_126_250))));
358+
359+
Spi::connect(|client| {
360+
let table =
361+
client.select("SELECT arr FROM flat_array_toast_test", Some(1), &[])?.first();
362+
let datum = table.get_datum_by_ordinal(1)?.expect("array was null");
363+
assert!(unsafe { pgrx::varlena::varatt_is_1b_e(datum.cast_mut_ptr()) });
364+
365+
unsafe {
366+
PgMemoryContexts::Transient {
367+
parent: PgMemoryContexts::CurrentMemoryContext.value(),
368+
name: "toasted FlatArray cleanup test",
369+
min_context_size: 8 * 1024,
370+
initial_block_size: 8 * 1024,
371+
max_block_size: 8 * 1024,
372+
}
373+
.switch_to(|context| {
374+
let call = || {
375+
direct_pg_extern_function_call::<i32>(
376+
super::borrow_get_arr_nelems_wrapper,
377+
&[Some(datum)],
378+
)
379+
};
380+
381+
assert_eq!(call(), Some(2500));
382+
let warmed = pg_sys::MemoryContextMemAllocated(context.value(), true);
383+
for _ in 0..64 {
384+
assert_eq!(call(), Some(2500));
385+
}
386+
let after = pg_sys::MemoryContextMemAllocated(context.value(), true);
387+
388+
assert!(
389+
after <= warmed + 64 * 1024,
390+
"detoasted arguments accumulated {} bytes",
391+
after - warmed
392+
);
393+
});
394+
}
395+
Ok(())
396+
})
397+
}
398+
333399
#[pg_test]
334400
fn borrow_test_get_arr_data_ptr_nth_elem() {
335401
let nth =

pgrx/src/array/flat_array.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,7 @@ fn alloc_zeroed_head(
316316

317317
unsafe impl<T: ?Sized> BorrowDatum for FlatArray<'_, T> {
318318
const PASS: layout::PassBy = layout::PassBy::Ref;
319+
319320
unsafe fn point_from(ptr: ptr::NonNull<u8>) -> ptr::NonNull<Self> {
320321
unsafe {
321322
let len =
@@ -325,6 +326,22 @@ unsafe impl<T: ?Sized> BorrowDatum for FlatArray<'_, T> {
325326
)
326327
}
327328
}
329+
330+
unsafe fn borrow_arg_unchecked<'dat>(
331+
ptr: ptr::NonNull<u8>,
332+
register: impl FnOnce(ptr::NonNull<u8>),
333+
) -> &'dat Self {
334+
// SAFETY: The caller guarantees `ptr` points to a valid PostgreSQL array Datum.
335+
let detoasted = unsafe { pg_sys::pg_detoast_datum(ptr.as_ptr().cast()) };
336+
let detoasted = ptr::NonNull::new(detoasted).expect("pg_detoast_datum returned null");
337+
if detoasted.cast() != ptr {
338+
register(detoasted.cast());
339+
}
340+
341+
// SAFETY: `pg_detoast_datum` returns an aligned, contiguous, initialized ArrayType, and a
342+
// fresh allocation is registered above to live until the function result has been boxed.
343+
unsafe { Self::point_from(detoasted.cast()).as_ref() }
344+
}
328345
}
329346

330347
/// `T[]` in Postgres

0 commit comments

Comments
 (0)