From d2095c96f2e10265be13f59c64413759111235b0 Mon Sep 17 00:00:00 2001 From: Jubilee <46493976+workingjubilee@users.noreply.github.com> Date: Wed, 8 Nov 2023 14:18:35 -0800 Subject: [PATCH] Allow safe u32 -> Oid conversions (#1374) A while back Thom discovered this, but we didn't open the PR at the time. Simply cast `u32 as i32` and pass it through SPI to do this by other means. While it is an incredibly bad idea to do so, unfortunately that means we cannot rely on the conversion being `unsafe`, as there is no way we would make SPI unsafe. That leaves no persuasive argument for not providing From. --- pgrx-pg-sys/src/submodules/oids.rs | 8 ++++++++ pgrx/src/datum/from.rs | 6 +----- pgrx/src/pgbox.rs | 2 +- pgrx/src/rel.rs | 4 ++-- pgrx/src/tupdesc.rs | 2 +- 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/pgrx-pg-sys/src/submodules/oids.rs b/pgrx-pg-sys/src/submodules/oids.rs index d873eecd9..151b28709 100644 --- a/pgrx-pg-sys/src/submodules/oids.rs +++ b/pgrx-pg-sys/src/submodules/oids.rs @@ -63,6 +63,7 @@ impl Oid { /// nor cite SQL statements for misdemeanors, nor even truly stop you from foolishness. /// Even "trustworthy" is meant here in a similar sense to how raw pointers can be "trustworthy". /// Often, you should still check if it's null. + #[deprecated(since = "0.12.0", note = "safely converts via SPI, use pg_sys::Oid::from(u32)")] pub const unsafe fn from_u32_unchecked(id: u32) -> Oid { Oid(id) } @@ -98,6 +99,13 @@ impl fmt::Display for Oid { } } +/// De facto available via SPI +impl From for Oid { + fn from(word: u32) -> Oid { + Oid(word) + } +} + impl From for u32 { fn from(oid: Oid) -> u32 { oid.0 diff --git a/pgrx/src/datum/from.rs b/pgrx/src/datum/from.rs index 24c7a47d9..b05be6790 100644 --- a/pgrx/src/datum/from.rs +++ b/pgrx/src/datum/from.rs @@ -196,11 +196,7 @@ impl FromDatum for pg_sys::Oid { if is_null { None } else { - datum - .value() - .try_into() - .ok() - .map(|uint| unsafe { pg_sys::Oid::from_u32_unchecked(uint) }) + datum.value().try_into().ok().map(|uint: u32| pg_sys::Oid::from(uint)) } } } diff --git a/pgrx/src/pgbox.rs b/pgrx/src/pgbox.rs index 139deb573..aa720d041 100644 --- a/pgrx/src/pgbox.rs +++ b/pgrx/src/pgbox.rs @@ -73,7 +73,7 @@ use std::ptr::NonNull; /// use pgrx::prelude::*; /// /// pub fn do_something() { -/// # let example_rel_oid = |i| { unsafe { pg_sys::Oid::from_u32_unchecked(i) } }; +/// # let example_rel_oid = |i| pg_sys::Oid::from(i); /// // open a relation and project it as a pg_sys::Relation /// let relid: pg_sys::Oid = example_rel_oid(42); /// let lockmode = pg_sys::AccessShareLock as i32; diff --git a/pgrx/src/rel.rs b/pgrx/src/rel.rs index 6442248d4..2cecf79b2 100644 --- a/pgrx/src/rel.rs +++ b/pgrx/src/rel.rs @@ -208,7 +208,7 @@ impl PgRelation { /// /// ```rust,no_run /// use pgrx::{PgRelation, pg_sys}; - /// # let example_pg_class_oid = |i| { unsafe { pg_sys::Oid::from_u32_unchecked(i) } }; + /// # let example_pg_class_oid = |i| pg_sys::Oid::from(i); /// let oid = example_pg_class_oid(42); // a valid pg_class "oid" value /// let relation = unsafe { PgRelation::from_pg(pg_sys::RelationIdGetRelation(oid) ) }; /// let tupdesc = relation.tuple_desc(); @@ -309,7 +309,7 @@ impl FromDatum for PgRelation { None } else { Some(PgRelation::with_lock( - unsafe { pg_sys::Oid::from_u32_unchecked(u32::try_from(datum.value()).ok()?) }, + pg_sys::Oid::from(u32::try_from(datum.value()).ok()?), pg_sys::AccessShareLock as pg_sys::LOCKMODE, )) } diff --git a/pgrx/src/tupdesc.rs b/pgrx/src/tupdesc.rs index a8f5fed3f..25578cdbe 100644 --- a/pgrx/src/tupdesc.rs +++ b/pgrx/src/tupdesc.rs @@ -116,7 +116,7 @@ impl<'a> PgTupleDesc<'a> { /// /// ```rust,no_run /// use pgrx::{pg_sys, PgTupleDesc}; - /// # let example_pg_type_oid = |i| { unsafe { pg_sys::Oid::from_u32_unchecked(i) } }; + /// # let example_pg_type_oid = |i| pg_sys::Oid::from(i); /// let typid = example_pg_type_oid(42); // a valid pg_type Oid /// let typmod = 0; // its corresponding typemod value /// let tupdesc = unsafe { PgTupleDesc::from_pg_is_copy(pg_sys::lookup_rowtype_tupdesc_copy(typid, typmod)) };