Skip to content

Commit

Permalink
Allow safe u32 -> Oid conversions (#1374)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
workingjubilee authored Nov 8, 2023
1 parent 2df66d8 commit 273dcdc
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 9 deletions.
8 changes: 8 additions & 0 deletions pgrx-pg-sys/src/submodules/oids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -98,6 +99,13 @@ impl fmt::Display for Oid {
}
}

/// De facto available via SPI
impl From<u32> for Oid {
fn from(word: u32) -> Oid {
Oid(word)
}
}

impl From<Oid> for u32 {
fn from(oid: Oid) -> u32 {
oid.0
Expand Down
6 changes: 1 addition & 5 deletions pgrx/src/datum/from.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,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))
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion pgrx/src/pgbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions pgrx/src/rel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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,
))
}
Expand Down
2 changes: 1 addition & 1 deletion pgrx/src/tupdesc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)) };
Expand Down

0 comments on commit 273dcdc

Please sign in to comment.