Skip to content

Commit 1ab34bd

Browse files
authored
do some cleanups around PgHeapTuple to make them easier to use (#1153)
Also addresses a safety issue by marking a function `unsafe`.
1 parent 5c17027 commit 1ab34bd

File tree

3 files changed

+60
-18
lines changed

3 files changed

+60
-18
lines changed

pgrx-tests/src/tests/heap_tuple.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -600,16 +600,18 @@ mod sql_generator_tests {
600600
#[pg_extern]
601601
fn generate_lots_of_dogs() -> SetOfIterator<'static, pgrx::composite_type!("Dog")> {
602602
let tuple_desc =
603-
pgrx::PgTupleDesc::for_composite_type("Dog").expect("Coudln't find TestType");
603+
pgrx::PgTupleDesc::for_composite_type("Dog").expect("Couldn't find TestType");
604604

605605
let tuples: Vec<PgHeapTuple<'_, AllocatedByRust>> = (0..10_000)
606606
.into_iter()
607607
.map(move |i| {
608608
let datums: Vec<Option<pg_sys::Datum>> =
609609
vec!["good boy".into_datum(), i.into_datum()];
610610

611-
PgHeapTuple::from_datums(tuple_desc.clone(), datums)
612-
.expect("couldn't get heap tuple")
611+
unsafe {
612+
PgHeapTuple::from_datums(tuple_desc.clone(), datums)
613+
.expect("couldn't get heap tuple")
614+
}
613615
})
614616
.collect();
615617

pgrx/src/heap_tuple.rs

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ use crate::{
99
FromDatum, IntoDatum, PgBox, PgMemoryContexts, PgTupleDesc, TriggerTuple, TryFromDatumError,
1010
WhoAllocated,
1111
};
12+
use pgrx_pg_sys::errcodes::PgSqlErrorCode;
13+
use pgrx_pg_sys::PgTryBuilder;
1214
use pgrx_sql_entity_graph::metadata::{
1315
ArgumentError, Returns, ReturnsError, SqlMapping, SqlTranslatable,
1416
};
@@ -22,6 +24,12 @@ pub enum PgHeapTupleError {
2224

2325
#[error("The specified composite type, {0}, does not exist")]
2426
NoSuchType(String),
27+
28+
#[error("The specified composite type, {0}, does not exist")]
29+
NoSuchTypeOid(pg_sys::Oid),
30+
31+
#[error("Oid `{0}` is not a composite type")]
32+
NotACompositeType(pg_sys::Oid),
2533
}
2634

2735
/// A [`PgHeapTuple`] is a lightweight wrapper around Postgres' [`pg_sys::HeapTuple`] object and a [`PgTupleDesc`].
@@ -199,28 +207,52 @@ impl<'a> PgHeapTuple<'a, AllocatedByRust> {
199207
) -> Result<PgHeapTuple<'a, AllocatedByRust>, PgHeapTupleError> {
200208
let tuple_desc = PgTupleDesc::for_composite_type(type_name)
201209
.ok_or_else(|| PgHeapTupleError::NoSuchType(type_name.to_string()))?;
202-
let natts = tuple_desc.len();
203-
unsafe {
204-
let datums =
205-
pg_sys::palloc0(natts * std::mem::size_of::<pg_sys::Datum>()) as *mut pg_sys::Datum;
206-
let mut is_null = (0..natts).map(|_| true).collect::<Vec<_>>();
207210

208-
let heap_tuple =
209-
pg_sys::heap_form_tuple(tuple_desc.as_ptr(), datums, is_null.as_mut_ptr());
211+
Self::new_composite_type_by_oid(tuple_desc.oid())
212+
}
210213

211-
Ok(PgHeapTuple {
212-
tuple: PgBox::<pg_sys::HeapTupleData, AllocatedByRust>::from_rust(heap_tuple),
213-
tupdesc: tuple_desc,
214-
})
215-
}
214+
pub fn new_composite_type_by_oid(
215+
typoid: pg_sys::Oid,
216+
) -> Result<PgHeapTuple<'a, AllocatedByRust>, PgHeapTupleError> {
217+
PgTryBuilder::new(|| {
218+
let tuple_desc = PgTupleDesc::for_composite_type_by_oid(typoid)
219+
.ok_or_else(|| PgHeapTupleError::NotACompositeType(typoid))?;
220+
let natts = tuple_desc.len();
221+
222+
unsafe {
223+
let datums = pg_sys::palloc0(natts * std::mem::size_of::<pg_sys::Datum>())
224+
as *mut pg_sys::Datum;
225+
let mut is_null = vec![true; natts];
226+
227+
let heap_tuple =
228+
pg_sys::heap_form_tuple(tuple_desc.as_ptr(), datums, is_null.as_mut_ptr());
229+
230+
Ok(PgHeapTuple {
231+
tuple: PgBox::<pg_sys::HeapTupleData, AllocatedByRust>::from_rust(heap_tuple),
232+
tupdesc: tuple_desc,
233+
})
234+
}
235+
})
236+
.catch_when(PgSqlErrorCode::ERRCODE_WRONG_OBJECT_TYPE, |_| {
237+
Err(PgHeapTupleError::NotACompositeType(typoid))
238+
})
239+
.catch_when(PgSqlErrorCode::ERRCODE_UNDEFINED_OBJECT, |_| {
240+
Err(PgHeapTupleError::NoSuchTypeOid(typoid))
241+
})
242+
.execute()
216243
}
217244

218245
/// Create a new [PgHeapTuple] from a [PgTupleDesc] from an iterator of Datums.
219246
///
220247
/// ## Errors
221248
/// - [PgHeapTupleError::IncorrectAttributeCount] if the number of items in the iterator
222249
/// does not match the number of attributes in the [PgTupleDesc].
223-
pub fn from_datums<I: IntoIterator<Item = Option<pg_sys::Datum>>>(
250+
///
251+
/// # Safety
252+
///
253+
/// This function is unsafe as we cannot guarantee the provided [`pg_sys::Datum`]s are valid
254+
/// as the specified [`PgTupleDesc`] might expect
255+
pub unsafe fn from_datums<I: IntoIterator<Item = Option<pg_sys::Datum>>>(
224256
tupdesc: PgTupleDesc<'a>,
225257
datums: I,
226258
) -> Result<PgHeapTuple<'a, AllocatedByRust>, PgHeapTupleError> {

pgrx/src/tupdesc.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ impl<'a> PgTupleDesc<'a> {
156156
```rust,no_run
157157
use pgrx::{prelude::*, PgTupleDesc};
158158
159-
Spi::run("CREATE TYPE Dog AS (name text, age int);");
159+
Spi::run("CREATE TYPE Dog AS (name text, age int);").unwrap();
160160
let tuple_desc = PgTupleDesc::for_composite_type("Dog").unwrap();
161161
let natts = tuple_desc.len();
162162
@@ -179,12 +179,20 @@ impl<'a> PgTupleDesc<'a> {
179179
let mut typmod = 0;
180180
pg_sys::parseTypeString(name.as_pg_cstr(), &mut typoid, &mut typmod, true);
181181

182+
Self::for_composite_type_by_oid(typoid)
183+
}
184+
}
185+
186+
/// Similar to [`PgTupleDesc::for_composite_type()`] but using the type's [`pg_sys::Oid`] instead
187+
/// of its name.
188+
pub fn for_composite_type_by_oid(typoid: pg_sys::Oid) -> Option<PgTupleDesc<'a>> {
189+
unsafe {
182190
if typoid == pg_sys::InvalidOid {
183191
return None;
184192
}
185193

186194
// It's important to make a copy of the tupledesc: https://www.postgresql.org/message-id/flat/24471.1136768659%40sss.pgh.pa.us
187-
let tuple_desc = pg_sys::lookup_rowtype_tupdesc_copy(typoid, typmod);
195+
let tuple_desc = pg_sys::lookup_rowtype_tupdesc_copy(typoid, -1);
188196

189197
Some(PgTupleDesc::from_pg_copy(tuple_desc))
190198
}

0 commit comments

Comments
 (0)