pg_sys::Oid::from_datum() should cast to u32 (#1991)

As implemented in Postgres, the OID version of a `Datum` is a cast of the Datum's `uintptr_t` value to a `u32`, not a bounds checked version of that pointer value.

Related, the `PgRelation::from_datum()` implementation is actually a REGCLASS behind the scenes, which is an OID, and it was similarly bugged.  This is kinda hard to write a test for as you'd need a relation of some kind in the schema with an OID over i32::MAX, and that's really tough to materialize without mucking around with the `pg_resetwal` tool, which is way outside the scope of what our test suite can handle.

Author: eeeebbbbrrrr

pgrx-tests/src/tests/mod.rs

@@ -43,6 +43,7 @@ mod log_tests;
 mod memcxt_tests;
 mod name_tests;
 mod numeric_tests;
+mod oid_tests;
 mod pg_cast_tests;
 mod pg_extern_tests;
 mod pg_guard_tests;

pgrx-tests/src/tests/oid_tests.rs

@@ -0,0 +1,39 @@
+//LICENSE Portions Copyright 2019-2021 ZomboDB, LLC.
+//LICENSE
+//LICENSE Portions Copyright 2021-2023 Technology Concepts & Design, Inc.
+//LICENSE
+//LICENSE Portions Copyright 2023-2023 PgCentral Foundation, Inc. <[email protected]>
+//LICENSE
+//LICENSE All rights reserved.
+//LICENSE
+//LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file.
+#[cfg(any(test, feature = "pg_test"))]
+#[pgrx::pg_schema]
+mod tests {
+    #[allow(unused_imports)]
+    use crate as pgrx_tests;
+
+    use pgrx::prelude::*;
+
+    #[pg_extern]
+    fn oid_roundtrip(oid: pg_sys::Oid) -> pg_sys::Oid {
+        oid
+    }
+
+    #[pg_test]
+    fn test_reasonable_oid() -> spi::Result<()> {
+        let oid = Spi::get_one::<pg_sys::Oid>("SELECT tests.oid_roundtrip(42)")?
+            .expect("SPI result was null");
+        assert_eq!(oid.as_u32(), 42);
+        Ok(())
+    }
+
+    #[pg_test]
+    fn test_completely_unreasonable_but_still_valid_oid() -> spi::Result<()> {
+        // nb:  this stupid value is greater than i32::MAX
+        let oid = Spi::get_one::<pg_sys::Oid>("SELECT tests.oid_roundtrip(2147483648)")?
+            .expect("SPI result was null");
+        assert_eq!(oid.as_u32(), 2_147_483_648);
+        Ok(())
+    }
+}

pgrx/src/datum/from.rs

@@ -188,7 +188,23 @@ impl FromDatum for pg_sys::Oid {
         if is_null {
             None
         } else {
-            datum.value().try_into().ok().map(|uint: u32| pg_sys::Oid::from(uint))
+            // NB:  Postgres' `DatumGetObjectId()` function is defined as a straight cast
+            // rather than assuming the Datum's pointer value is itself a valid unsigned int:
+            //
+            // ```c
+            // /*
+            //  * DatumGetObjectId
+            //  *		Returns object identifier value of a datum.
+            //  */
+            // static inline Oid
+            // DatumGetObjectId(Datum X)
+            // {
+            // 	return (Oid) X;
+            // }
+            // ```
+
+            let oid_as_u32 = datum.value() as u32;
+            Some(pg_sys::Oid::from(oid_as_u32))
         }
     }
 }

pgrx/src/rel.rs

@@ -308,10 +308,10 @@ impl FromDatum for PgRelation {
         if is_null {
             None
         } else {
-            Some(PgRelation::with_lock(
-                pg_sys::Oid::from(u32::try_from(datum.value()).ok()?),
-                pg_sys::AccessShareLock as pg_sys::LOCKMODE,
-            ))
+            // the `PgRelation` SQL type is `REGCLASS`, which is just an `OID`, so that's how
+            // we'll get the value.
+            let oid = pg_sys::Oid::from_datum(datum, false)?;
+            Some(PgRelation::with_lock(oid, pg_sys::AccessShareLock as pg_sys::LOCKMODE))
         }
     }
 }