From 2ed0cb12f2ae21053e6a7c7dd358bcf0b534efd6 Mon Sep 17 00:00:00 2001 From: Jubilee <46493976+workingjubilee@users.noreply.github.com> Date: Fri, 7 Oct 2022 12:40:48 -0700 Subject: [PATCH] Require `FromDatum::from_polymorphic_datum` (#749) * Require `FromDatum::from_polymorphic_datum` This also implies defaulting `FromDatum::from_datum` which just calls `FromDatum::from_polymorphic_datum` with `pg_sys::InvalidOid`. Polymorphic types like AnyElement should generally override this on implementing, and non-polymorphic types should ignore the third parameter, while others may want a pass-through behavior. --- pgx-examples/bgworker/src/lib.rs | 2 +- pgx-macros/src/lib.rs | 2 +- pgx/src/datum/anyarray.rs | 11 +- pgx/src/datum/anyelement.rs | 11 +- pgx/src/datum/array.rs | 16 +-- pgx/src/datum/date.rs | 6 +- pgx/src/datum/from.rs | 119 ++++++++++++++++------ pgx/src/datum/geo.rs | 12 ++- pgx/src/datum/inet.rs | 6 +- pgx/src/datum/internal.rs | 6 +- pgx/src/datum/item_pointer_data.rs | 2 +- pgx/src/datum/json.rs | 14 ++- pgx/src/datum/numeric.rs | 6 +- pgx/src/datum/time.rs | 6 +- pgx/src/datum/time_stamp.rs | 6 +- pgx/src/datum/time_stamp_with_timezone.rs | 6 +- pgx/src/datum/time_with_timezone.rs | 4 +- pgx/src/datum/tuples.rs | 28 +++-- pgx/src/datum/uuid.rs | 2 +- pgx/src/datum/varlena.rs | 12 ++- pgx/src/fcinfo.rs | 8 +- pgx/src/heap_tuple.rs | 2 +- pgx/src/htup.rs | 4 +- pgx/src/rel.rs | 6 +- pgx/src/spi.rs | 8 +- 25 files changed, 215 insertions(+), 90 deletions(-) diff --git a/pgx-examples/bgworker/src/lib.rs b/pgx-examples/bgworker/src/lib.rs index d11462fe8..fd312a82a 100644 --- a/pgx-examples/bgworker/src/lib.rs +++ b/pgx-examples/bgworker/src/lib.rs @@ -44,7 +44,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_polymorphic_datum(arg, false, pg_sys::INT4OID) }; // 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 diff --git a/pgx-macros/src/lib.rs b/pgx-macros/src/lib.rs index 9f35262cb..95a5e00e1 100644 --- a/pgx-macros/src/lib.rs +++ b/pgx-macros/src/lib.rs @@ -621,7 +621,7 @@ fn impl_postgres_enum(ast: DeriveInput) -> proc_macro2::TokenStream { stream.extend(quote! { impl ::pgx::datum::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_polymorphic_datum(datum: ::pgx::pg_sys::Datum, is_null: bool, typeoid: ::pgx::pg_sys::Oid) -> Option<#enum_ident> { if is_null { None } else { diff --git a/pgx/src/datum/anyarray.rs b/pgx/src/datum/anyarray.rs index 70aad5b98..c5e92674d 100644 --- a/pgx/src/datum/anyarray.rs +++ b/pgx/src/datum/anyarray.rs @@ -29,7 +29,7 @@ impl AnyArray { #[inline] pub fn into(&self) -> Option { - unsafe { T::from_datum(self.datum(), false, self.oid()) } + unsafe { T::from_polymorphic_datum(self.datum(), false, self.oid()) } } } @@ -37,12 +37,9 @@ impl FromDatum for AnyArray { const GET_TYPOID: bool = true; #[inline] - unsafe fn from_datum( - datum: pg_sys::Datum, - is_null: bool, - typoid: pg_sys::Oid, - ) -> Option { - FromDatum::from_polymorphic_datum(datum, is_null, typoid) + unsafe fn from_datum(_datum: pg_sys::Datum, _is_null: bool) -> Option { + debug_assert!(false, "Can't create a polymorphic type using from_datum, call FromDatum::from_polymorphic_datum instead"); + None } #[inline] diff --git a/pgx/src/datum/anyelement.rs b/pgx/src/datum/anyelement.rs index d09b01ffe..fc9d101ad 100644 --- a/pgx/src/datum/anyelement.rs +++ b/pgx/src/datum/anyelement.rs @@ -29,7 +29,7 @@ impl AnyElement { #[inline] pub fn into(&self) -> Option { - unsafe { T::from_datum(self.datum(), false, self.oid()) } + unsafe { T::from_polymorphic_datum(self.datum(), false, self.oid()) } } } @@ -37,12 +37,9 @@ impl FromDatum for AnyElement { const GET_TYPOID: bool = true; #[inline] - unsafe fn from_datum( - datum: pg_sys::Datum, - is_null: bool, - typoid: pg_sys::Oid, - ) -> Option { - FromDatum::from_polymorphic_datum(datum, is_null, typoid) + unsafe fn from_datum(_datum: pg_sys::Datum, _is_null: bool) -> Option { + debug_assert!(false, "Can't create a polymorphic type using from_datum, call FromDatum::from_polymorphic_datum instead"); + None } #[inline] diff --git a/pgx/src/datum/array.rs b/pgx/src/datum/array.rs index ae5da05dc..163c49274 100644 --- a/pgx/src/datum/array.rs +++ b/pgx/src/datum/array.rs @@ -318,7 +318,7 @@ impl<'a, T: FromDatum> Array<'a, T> { None } else { Some(unsafe { - T::from_datum( + T::from_polymorphic_datum( self.elem_slice[i], self.null_slice.get(i)?, self.raw.as_ref().map(|r| r.oid()).unwrap_or_default(), @@ -502,18 +502,18 @@ impl<'a, T: FromDatum> Iterator for ArrayIntoIterator<'a, T> { impl<'a, T: FromDatum> FromDatum for VariadicArray<'a, T> { #[inline] - unsafe fn from_datum( + unsafe fn from_polymorphic_datum( datum: pg_sys::Datum, is_null: bool, oid: pg_sys::Oid, ) -> Option> { - Array::from_datum(datum, is_null, oid).map(Self) + Array::from_polymorphic_datum(datum, is_null, oid).map(Self) } } impl<'a, T: FromDatum> FromDatum for Array<'a, T> { #[inline] - unsafe fn from_datum( + unsafe fn from_polymorphic_datum( datum: pg_sys::Datum, is_null: bool, _typoid: u32, @@ -536,7 +536,7 @@ impl<'a, T: FromDatum> FromDatum for Array<'a, T> { impl FromDatum for Vec { #[inline] - unsafe fn from_datum( + unsafe fn from_polymorphic_datum( datum: pg_sys::Datum, is_null: bool, typoid: pg_sys::Oid, @@ -544,7 +544,7 @@ impl FromDatum for Vec { if is_null { None } else { - let array = Array::::from_datum(datum, is_null, typoid).unwrap(); + let array = Array::::from_polymorphic_datum(datum, is_null, typoid).unwrap(); let mut v = Vec::with_capacity(array.len()); for element in array.iter() { @@ -557,7 +557,7 @@ impl FromDatum for Vec { impl FromDatum for Vec> { #[inline] - unsafe fn from_datum( + unsafe fn from_polymorphic_datum( datum: pg_sys::Datum, is_null: bool, typoid: pg_sys::Oid, @@ -565,7 +565,7 @@ impl FromDatum for Vec> { if is_null || datum.is_null() { None } else { - let array = Array::::from_datum(datum, is_null, typoid).unwrap(); + let array = Array::::from_polymorphic_datum(datum, is_null, typoid).unwrap(); let mut v = Vec::with_capacity(array.len()); for element in array.iter() { diff --git a/pgx/src/datum/date.rs b/pgx/src/datum/date.rs index abe49ca87..29aa3fca9 100644 --- a/pgx/src/datum/date.rs +++ b/pgx/src/datum/date.rs @@ -38,7 +38,11 @@ impl IntoDatum for Date { } impl FromDatum for Date { - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option where Self: Sized, { diff --git a/pgx/src/datum/from.rs b/pgx/src/datum/from.rs index 36edcf328..7ec1ad692 100644 --- a/pgx/src/datum/from.rs +++ b/pgx/src/datum/from.rs @@ -53,9 +53,12 @@ pub trait FromDatum { /// /// If, however, you're providing an arbitrary datum value, it needs to be considered unsafe /// and that unsafeness should be propagated through your API. - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, typoid: pg_sys::Oid) -> Option + unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool) -> Option where - Self: Sized; + Self: Sized, + { + FromDatum::from_polymorphic_datum(datum, is_null, pg_sys::InvalidOid) + } /// Like `from_datum` for instantiating polymorphic types /// which require preserving the dynamic type metadata. @@ -69,10 +72,7 @@ pub trait FromDatum { typoid: pg_sys::Oid, ) -> Option where - Self: Sized, - { - FromDatum::from_datum(datum, is_null, typoid) - } + Self: Sized; /// Default implementation switched to the specified memory context and then simply calls /// `FromDatum::from_datum(...)` from within that context. @@ -96,7 +96,7 @@ pub trait FromDatum { where Self: Sized, { - memory_context.switch_to(|_| FromDatum::from_datum(datum, is_null, typoid)) + memory_context.switch_to(|_| FromDatum::from_polymorphic_datum(datum, is_null, typoid)) } /// `try_from_datum` is a convenience wrapper around `FromDatum::from_datum` that returns a @@ -120,7 +120,7 @@ pub trait FromDatum { } else if !is_null && datum.is_null() && !Self::is_pass_by_value() { Err(TryFromDatumError::NullDatumPointer) } else { - Ok(FromDatum::from_datum(datum, is_null, type_oid)) + Ok(FromDatum::from_polymorphic_datum(datum, is_null, type_oid)) } } } @@ -128,7 +128,7 @@ pub trait FromDatum { /// for pg_sys::Datum impl FromDatum for pg_sys::Datum { #[inline] - unsafe fn from_datum( + unsafe fn from_polymorphic_datum( datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid, @@ -144,7 +144,11 @@ impl FromDatum for pg_sys::Datum { /// for bool impl FromDatum for bool { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option { if is_null { None } else { @@ -156,7 +160,11 @@ impl FromDatum for bool { /// for `"char"` impl FromDatum for i8 { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option { if is_null { None } else { @@ -168,7 +176,11 @@ impl FromDatum for i8 { /// for smallint impl FromDatum for i16 { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option { if is_null { None } else { @@ -180,7 +192,11 @@ impl FromDatum for i16 { /// for integer impl FromDatum for i32 { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option { if is_null { None } else { @@ -192,7 +208,11 @@ impl FromDatum for i32 { /// for oid impl FromDatum for u32 { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option { if is_null { None } else { @@ -204,7 +224,11 @@ impl FromDatum for u32 { /// for bigint impl FromDatum for i64 { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option { if is_null { None } else { @@ -216,7 +240,11 @@ impl FromDatum for i64 { /// for real impl FromDatum for f32 { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option { if is_null { None } else { @@ -228,7 +256,11 @@ impl FromDatum for f32 { /// for double precision impl FromDatum for f64 { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option { if is_null { None } else { @@ -240,7 +272,11 @@ impl FromDatum for f64 { /// for text, varchar impl<'a> FromDatum for &'a str { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option<&'a str> { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option<&'a str> { if is_null || datum.is_null() { None } else { @@ -280,26 +316,35 @@ impl<'a> FromDatum for &'a str { /// This returns a **copy**, allocated and managed by Rust, of the underlying `varlena` Datum impl FromDatum for String { #[inline] - unsafe fn from_datum( + unsafe fn from_polymorphic_datum( datum: pg_sys::Datum, is_null: bool, typoid: pg_sys::Oid, ) -> Option { - FromDatum::from_datum(datum, is_null, typoid).map(|s: &str| s.to_owned()) + FromDatum::from_polymorphic_datum(datum, is_null, typoid).map(|s: &str| s.to_owned()) } } impl FromDatum for char { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, typoid: pg_sys::Oid) -> Option { - FromDatum::from_datum(datum, is_null, typoid).and_then(|s: &str| s.chars().next()) + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + typoid: pg_sys::Oid, + ) -> Option { + FromDatum::from_polymorphic_datum(datum, is_null, typoid) + .and_then(|s: &str| s.chars().next()) } } /// for cstring impl<'a> FromDatum for &'a std::ffi::CStr { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option<&'a CStr> { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option<&'a CStr> { if is_null || datum.is_null() { None } else { @@ -310,7 +355,7 @@ impl<'a> FromDatum for &'a std::ffi::CStr { impl<'a> FromDatum for &'a crate::cstr_core::CStr { #[inline] - unsafe fn from_datum( + unsafe fn from_polymorphic_datum( datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid, @@ -326,7 +371,11 @@ impl<'a> FromDatum for &'a crate::cstr_core::CStr { /// for bytea impl<'a> FromDatum for &'a [u8] { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _typoid: u32) -> Option<&'a [u8]> { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _typoid: u32, + ) -> Option<&'a [u8]> { if is_null || datum.is_null() { None } else { @@ -363,12 +412,16 @@ impl<'a> FromDatum for &'a [u8] { impl FromDatum for Vec { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, typoid: u32) -> Option> { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + typoid: u32, + ) -> Option> { if is_null || datum.is_null() { None } else { // Vec conversion is initially the same as for &[u8] - let bytes: Option<&[u8]> = FromDatum::from_datum(datum, is_null, typoid); + let bytes: Option<&[u8]> = FromDatum::from_polymorphic_datum(datum, is_null, typoid); match bytes { // but then we need to convert it into an owned Vec where the backing @@ -383,7 +436,11 @@ impl FromDatum for Vec { /// for NULL -- always converts to a `None`, even if the is_null argument is false impl FromDatum for () { #[inline] - unsafe fn from_datum(_datum: pg_sys::Datum, _is_null: bool, _: pg_sys::Oid) -> Option<()> { + unsafe fn from_polymorphic_datum( + _datum: pg_sys::Datum, + _is_null: bool, + _: pg_sys::Oid, + ) -> Option<()> { None } } @@ -391,7 +448,11 @@ impl FromDatum for () { /// for user types impl FromDatum for PgBox { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option { if is_null || datum.is_null() { None } else { diff --git a/pgx/src/datum/geo.rs b/pgx/src/datum/geo.rs index facdd0b6d..e7295fed9 100644 --- a/pgx/src/datum/geo.rs +++ b/pgx/src/datum/geo.rs @@ -10,7 +10,11 @@ Use of this source code is governed by the MIT license that can be found in the use crate::{direct_function_call_as_datum, pg_sys, FromDatum, IntoDatum}; impl FromDatum for pg_sys::BOX { - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option where Self: Sized, { @@ -40,7 +44,11 @@ impl IntoDatum for pg_sys::BOX { } impl FromDatum for pg_sys::Point { - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option where Self: Sized, { diff --git a/pgx/src/datum/inet.rs b/pgx/src/datum/inet.rs index 7c055a0fa..28535cd27 100644 --- a/pgx/src/datum/inet.rs +++ b/pgx/src/datum/inet.rs @@ -86,7 +86,11 @@ impl<'de> Deserialize<'de> for Inet { } impl FromDatum for Inet { - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _typoid: u32) -> Option { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _typoid: u32, + ) -> Option { if is_null { None } else { diff --git a/pgx/src/datum/internal.rs b/pgx/src/datum/internal.rs index 6c5167728..8445e27a7 100644 --- a/pgx/src/datum/internal.rs +++ b/pgx/src/datum/internal.rs @@ -164,7 +164,11 @@ impl From> for Internal { impl FromDatum for Internal { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option { Some(Internal(if is_null { None } else { Some(datum) })) } } diff --git a/pgx/src/datum/item_pointer_data.rs b/pgx/src/datum/item_pointer_data.rs index 8314ca518..bd3164669 100644 --- a/pgx/src/datum/item_pointer_data.rs +++ b/pgx/src/datum/item_pointer_data.rs @@ -13,7 +13,7 @@ use crate::{ impl FromDatum for pg_sys::ItemPointerData { #[inline] - unsafe fn from_datum( + unsafe fn from_polymorphic_datum( datum: pg_sys::Datum, is_null: bool, _typoid: u32, diff --git a/pgx/src/datum/json.rs b/pgx/src/datum/json.rs index 88394b1d4..829a81e0b 100644 --- a/pgx/src/datum/json.rs +++ b/pgx/src/datum/json.rs @@ -29,7 +29,11 @@ pub struct JsonString(pub String); /// for json impl FromDatum for Json { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option { if is_null { None } else { @@ -45,7 +49,11 @@ impl FromDatum for Json { /// for jsonb impl FromDatum for JsonB { - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid) -> Option { + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _: pg_sys::Oid, + ) -> Option { if is_null { None } else { @@ -83,7 +91,7 @@ impl FromDatum for JsonB { /// This returns a **copy**, allocated and managed by Rust, of the underlying `varlena` Datum impl FromDatum for JsonString { #[inline] - unsafe fn from_datum( + unsafe fn from_polymorphic_datum( datum: pg_sys::Datum, is_null: bool, _: pg_sys::Oid, diff --git a/pgx/src/datum/numeric.rs b/pgx/src/datum/numeric.rs index ec3fcc7f3..f23361de7 100644 --- a/pgx/src/datum/numeric.rs +++ b/pgx/src/datum/numeric.rs @@ -159,7 +159,11 @@ impl From for Numeric { } impl FromDatum for Numeric { - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _typoid: u32) -> Option + unsafe fn from_polymorphic_datum( + datum: pg_sys::Datum, + is_null: bool, + _typoid: u32, + ) -> Option where Self: Sized, { diff --git a/pgx/src/datum/time.rs b/pgx/src/datum/time.rs index a2900b325..9c6089d1f 100644 --- a/pgx/src/datum/time.rs +++ b/pgx/src/datum/time.rs @@ -26,7 +26,11 @@ pub(crate) const USECS_PER_DAY: u64 = USECS_PER_HOUR * 24; pub struct Time(pub u64 /* Microseconds since midnight */); impl FromDatum for Time { #[inline] - unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool, _typoid: u32) -> Option