Skip to content

Commit

Permalink
Improved "HeapTuple" support (#532)
Browse files Browse the repository at this point in the history
* Remove FromDatum::NEEDS_TYPID const

This constant was set to false in all every `FromDatum` implementation, and simply isn't necessary.

Further changes to `FromDatum` are going to change how this idea worked in the first place.

* Remove the `typid` argument from `FromDatum::from_datum()`.

This argument was never actually used, only passed around.  And in many cases it was set to `pg_sys::InvalidOid` anyways.

* Eliminate this pattern during `FromDatum::from_datum()`:

```rust
        } else if datum == 0 {
            panic!("array was flagged not null but datum is zero");
        }
```

As the function is already unsafe, there's no need for us to have a check for a case that is likely never going to happen.  Postgres may send us a null datum, but as long as we check `is_null` first, we're okay.

* `FromDatum` gets a new function, `try_from_datum()`

This function ensures the desired Rust type is compatible with the backing Postgres type of the Datum being converted.  If they're not compatible, an `Err` is returned rather than either panicking or leading to undefined behavior.

It's not actually used anywhere yet.

* Initial work on a new `PgHeapTuple` type.

Currently it only knows how to construct itself in a few ways, along with retrieving named/indexed datum values (as runtime type-checked Rust values).

There's a few breaking API changes in other places, especially in `htup.rs` and `PgTupleDesc`

* default impl of a new IntoDatum::is_pass_by_value() function

* fix compilation issues after merging from-datum-overhaul

* a few more pass-by-value types

* WIP: ability to set attribute values on a HeapTuple

* Finish up the `PgHeapTuple` API, plus docs.  Update the trigger example.
  • Loading branch information
eeeebbbbrrrr authored Apr 20, 2022
1 parent c4ce4b9 commit d130652
Show file tree
Hide file tree
Showing 30 changed files with 679 additions and 307 deletions.
2 changes: 1 addition & 1 deletion pgx-examples/bgworker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub extern "C" fn _PG_init() {
#[pg_guard]
#[no_mangle]
pub extern "C" fn background_worker_main(arg: pg_sys::Datum) {
let arg = unsafe { i32::from_datum(arg, false, pg_sys::INT4OID) };
let arg = unsafe { i32::from_datum(arg, false) };

// these are the signals we want to receive. If we don't attach the SIGTERM handler, then
// we'll never be able to exit via an external notification
Expand Down
43 changes: 32 additions & 11 deletions pgx-examples/triggers/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,29 @@ unsafe fn trigger_example(fcinfo: pg_sys::FunctionCallInfo) -> pg_sys::Datum {
panic!("not called by trigger manager");
}

let trigdata: PgBox<pg_sys::TriggerData> = PgBox::from_pg(
fcinfo.as_ref().expect("fcinfo is NULL").context as *mut pg_sys::TriggerData,
);
let trigdata = (fcinfo.as_ref().expect("fcinfo is NULL").context as *mut pg_sys::TriggerData)
.as_ref()
.unwrap();

// and for this example, we're only going to operate as an ON BEFORE INSERT FOR EACH ROW trigger
if trigger_fired_before(trigdata.tg_event)
&& trigger_fired_by_insert(trigdata.tg_event)
&& trigger_fired_for_row(trigdata.tg_event)
{
let tupdesc = PgTupleDesc::from_pg_copy(trigdata.tg_relation.as_ref().unwrap().rd_att);
let tuple = PgBox::<pg_sys::HeapTupleData>::from_pg(trigdata.tg_trigtuple);
let id = heap_getattr::<i64, AllocatedByPostgres>(&tuple, 1, &tupdesc);
let title = heap_getattr::<&str, AllocatedByPostgres>(&tuple, 2, &tupdesc);
let description = heap_getattr::<&str, AllocatedByPostgres>(&tuple, 3, &tupdesc);
let payload = heap_getattr::<JsonB, AllocatedByPostgres>(&tuple, 4, &tupdesc);
let tuple =
PgHeapTuple::from_trigger_data(trigdata, TriggerTuple::Current).expect("tuple is NULL");
let id = tuple
.get_by_index::<i64>(1.try_into().unwrap())
.expect("could not get id");
let title = tuple
.get_by_index::<String>(2.try_into().unwrap())
.expect("could not get title");
let description = tuple
.get_by_index::<String>(3.try_into().unwrap())
.expect("could not get description");
let payload = tuple
.get_by_index::<JsonB>(4.try_into().unwrap())
.expect("could not get payload");

warning!(
"id={:?}, title={:?}, description={:?}, payload={:?}",
Expand All @@ -47,8 +55,21 @@ unsafe fn trigger_example(fcinfo: pg_sys::FunctionCallInfo) -> pg_sys::Datum {
payload
);

// return the inserting tuple, unchanged
trigdata.tg_trigtuple as pg_sys::Datum
// change the title
let mut tuple = tuple.into_owned();
tuple
.set_by_name("title", "a new title")
.expect("failed to change the title");
assert_eq!(
tuple.get_by_name::<&str>("title").unwrap().unwrap(),
"a new title"
);

// return the inserting tuple, which includes the changed title
match tuple.into_datum() {
Some(datum) => datum,
None => return pg_return_null(fcinfo),
}
} else {
panic!("not fired in the ON BEFORE INSERT context");
}
Expand Down
2 changes: 1 addition & 1 deletion pgx-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ fn impl_postgres_enum(ast: DeriveInput) -> proc_macro2::TokenStream {
stream.extend(quote! {
impl pgx::FromDatum for #enum_ident {
#[inline]
unsafe fn from_datum(datum: pgx::pg_sys::Datum, is_null: bool, typeoid: pgx::pg_sys::Oid) -> Option<#enum_ident> {
unsafe fn from_datum(datum: pgx::pg_sys::Datum, is_null: bool) -> Option<#enum_ident> {
if is_null {
None
} else {
Expand Down
15 changes: 3 additions & 12 deletions pgx/src/datum/anyarray.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,35 +12,26 @@ use crate::{pg_sys, FromDatum, IntoDatum};
#[derive(Debug, Clone, Copy)]
pub struct AnyArray {
datum: pg_sys::Datum,
typoid: pg_sys::Oid,
}

impl AnyArray {
pub fn datum(&self) -> pg_sys::Datum {
self.datum
}

pub fn oid(&self) -> pg_sys::Oid {
self.typoid
}

#[inline]
pub fn into<T: FromDatum>(&self) -> Option<T> {
unsafe { T::from_datum(self.datum(), false, self.oid()) }
unsafe { T::from_datum(self.datum(), false) }
}
}

impl FromDatum for AnyArray {
#[inline]
unsafe fn from_datum(
datum: pg_sys::Datum,
is_null: bool,
typoid: pg_sys::Oid,
) -> Option<AnyArray> {
unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool) -> Option<AnyArray> {
if is_null {
None
} else {
Some(AnyArray { datum, typoid })
Some(AnyArray { datum })
}
}
}
Expand Down
15 changes: 3 additions & 12 deletions pgx/src/datum/anyelement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,35 +12,26 @@ use crate::{pg_sys, FromDatum, IntoDatum};
#[derive(Debug, Clone, Copy)]
pub struct AnyElement {
datum: pg_sys::Datum,
typoid: pg_sys::Oid,
}

impl AnyElement {
pub fn datum(&self) -> pg_sys::Datum {
self.datum
}

pub fn oid(&self) -> pg_sys::Oid {
self.typoid
}

#[inline]
pub fn into<T: FromDatum>(&self) -> Option<T> {
unsafe { T::from_datum(self.datum(), false, self.oid()) }
unsafe { T::from_datum(self.datum(), false) }
}
}

impl FromDatum for AnyElement {
#[inline]
unsafe fn from_datum(
datum: pg_sys::Datum,
is_null: bool,
typoid: pg_sys::Oid,
) -> Option<AnyElement> {
unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool) -> Option<AnyElement> {
if is_null {
None
} else {
Some(AnyElement { datum, typoid })
Some(AnyElement { datum })
}
}
}
Expand Down
51 changes: 18 additions & 33 deletions pgx/src/datum/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ pub struct Array<'a, T: FromDatum> {
array_type: *mut pg_sys::ArrayType,
elements: *mut pg_sys::Datum,
nulls: *mut bool,
typoid: pg_sys::Oid,
nelems: usize,
elem_slice: &'a [pg_sys::Datum],
null_slice: &'a [bool],
Expand Down Expand Up @@ -63,7 +62,6 @@ impl<'a, T: FromDatum> Array<'a, T> {
array_type: std::ptr::null_mut(),
elements,
nulls,
typoid: pg_sys::InvalidOid,
nelems,
elem_slice: std::slice::from_raw_parts(elements, nelems),
null_slice: std::slice::from_raw_parts(nulls, nelems),
Expand All @@ -76,15 +74,13 @@ impl<'a, T: FromDatum> Array<'a, T> {
array_type: *mut pg_sys::ArrayType,
elements: *mut pg_sys::Datum,
nulls: *mut bool,
typoid: pg_sys::Oid,
nelems: usize,
) -> Self {
Array::<T> {
ptr,
array_type,
elements,
nulls,
typoid,
nelems,
elem_slice: std::slice::from_raw_parts(elements, nelems),
null_slice: std::slice::from_raw_parts(nulls, nelems),
Expand Down Expand Up @@ -153,7 +149,7 @@ impl<'a, T: FromDatum> Array<'a, T> {
if i >= self.nelems {
None
} else {
Some(unsafe { T::from_datum(self.elem_slice[i], self.null_slice[i], self.typoid) })
Some(unsafe { T::from_datum(self.elem_slice[i], self.null_slice[i]) })
}
}
}
Expand Down Expand Up @@ -277,11 +273,9 @@ impl<'a, T: FromDatum> Drop for Array<'a, T> {

impl<'a, T: FromDatum> FromDatum for Array<'a, T> {
#[inline]
unsafe fn from_datum(datum: usize, is_null: bool, typoid: u32) -> Option<Array<'a, T>> {
unsafe fn from_datum(datum: usize, is_null: bool) -> Option<Array<'a, T>> {
if is_null {
None
} else if datum == 0 {
panic!("array was flagged not null but datum is zero");
} else {
let ptr = datum as *mut pg_sys::varlena;
let array =
Expand Down Expand Up @@ -316,31 +310,18 @@ impl<'a, T: FromDatum> FromDatum for Array<'a, T> {
&mut nelems,
);

Some(Array::from_pg(
ptr,
array,
elements,
nulls,
typoid,
nelems as usize,
))
Some(Array::from_pg(ptr, array, elements, nulls, nelems as usize))
}
}
}

impl<T: FromDatum> FromDatum for Vec<T> {
#[inline]
unsafe fn from_datum(
datum: pg_sys::Datum,
is_null: bool,
typoid: pg_sys::Oid,
) -> Option<Vec<T>> {
unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool) -> Option<Vec<T>> {
if is_null {
None
} else if datum == 0 {
panic!("array was flagged not null but datum is zero");
} else {
let array = Array::<T>::from_datum(datum, is_null, typoid).unwrap();
let array = Array::<T>::from_datum(datum, is_null).unwrap();
let mut v = Vec::with_capacity(array.len());

for element in array.iter() {
Expand All @@ -353,17 +334,11 @@ impl<T: FromDatum> FromDatum for Vec<T> {

impl<T: FromDatum> FromDatum for Vec<Option<T>> {
#[inline]
unsafe fn from_datum(
datum: pg_sys::Datum,
is_null: bool,
typoid: pg_sys::Oid,
) -> Option<Vec<Option<T>>> {
unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool) -> Option<Vec<Option<T>>> {
if is_null {
None
} else if datum == 0 {
panic!("array was flagged not null but datum is zero");
} else {
let array = Array::<T>::from_datum(datum, is_null, typoid).unwrap();
let array = Array::<T>::from_datum(datum, is_null).unwrap();
let mut v = Vec::with_capacity(array.len());

for element in array.iter() {
Expand Down Expand Up @@ -414,11 +389,16 @@ where
fn type_oid() -> u32 {
unsafe { pg_sys::get_array_type(T::type_oid()) }
}

#[inline]
fn is_compatible_with(other: pg_sys::Oid) -> bool {
Self::type_oid() == other || other == unsafe { pg_sys::get_array_type(T::type_oid()) }
}
}

impl<'a, T> IntoDatum for &'a [T]
where
T: IntoDatum + Copy,
T: IntoDatum + Copy + 'a,
{
fn into_datum(self) -> Option<pg_sys::Datum> {
let mut state = unsafe {
Expand Down Expand Up @@ -456,4 +436,9 @@ where
fn type_oid() -> u32 {
unsafe { pg_sys::get_array_type(T::type_oid()) }
}

#[inline]
fn is_compatible_with(other: pg_sys::Oid) -> bool {
Self::type_oid() == other || other == unsafe { pg_sys::get_array_type(T::type_oid()) }
}
}
3 changes: 1 addition & 2 deletions pgx/src/datum/date.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ use time::format_description::FormatItem;
#[derive(Debug)]
pub struct Date(time::Date);
impl FromDatum for Date {
const NEEDS_TYPID: bool = false;
#[inline]
unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _typoid: u32) -> Option<Date> {
unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool) -> Option<Date> {
if is_null {
None
} else {
Expand Down
Loading

0 comments on commit d130652

Please sign in to comment.