From f43969a8244fa77cc4dbc440a64bf555d9b2fd9b Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Thu, 12 Sep 2024 22:52:54 +0300 Subject: [PATCH 1/7] Fixed conversion of i64 and f64 to datums on 32-bit machines --- pgrx-pg-sys/src/submodules/datum.rs | 12 ++++++++++-- pgrx/src/datum/from.rs | 16 +++++++++++++--- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/pgrx-pg-sys/src/submodules/datum.rs b/pgrx-pg-sys/src/submodules/datum.rs index 41326f1cf5..5252d9a1df 100644 --- a/pgrx-pg-sys/src/submodules/datum.rs +++ b/pgrx-pg-sys/src/submodules/datum.rs @@ -129,7 +129,11 @@ impl From for Datum { impl From for Datum { #[inline] fn from(val: u64) -> Datum { - Datum::from(val as usize) + if size_of::() <= size_of::() { + Datum::from(val as usize) + } else { + Datum::from(Box::into_raw(Box::new(val))) + } } } @@ -157,7 +161,11 @@ impl From for Datum { impl From for Datum { #[inline] fn from(val: i64) -> Datum { - Datum::from(val as usize) + if size_of::() <= size_of::() { + Datum::from(val as usize) + } else { + Datum::from(Box::into_raw(Box::new(val))) + } } } diff --git a/pgrx/src/datum/from.rs b/pgrx/src/datum/from.rs index ff2b52940e..171f28f918 100644 --- a/pgrx/src/datum/from.rs +++ b/pgrx/src/datum/from.rs @@ -12,7 +12,7 @@ use crate::{ pg_sys, varlena, varlena_to_byte_slice, AllocatedByPostgres, IntoDatum, PgBox, PgMemoryContexts, }; -use core::ffi::CStr; +use core::{ffi::CStr, mem::size_of}; use std::num::NonZeroUsize; /// If converting a Datum to a Rust type fails, this is the set of possible reasons why. @@ -283,7 +283,12 @@ impl FromDatum for i64 { if is_null { None } else { - Some(datum.value() as _) + let value = if size_of::() <= size_of::() { + datum.value() as _ + } else { + *(datum.cast_mut_ptr() as *const _) + }; + Some(value) } } } @@ -315,7 +320,12 @@ impl FromDatum for f64 { if is_null { None } else { - Some(f64::from_bits(datum.value() as _)) + let value = if size_of::() <= size_of::() { + f64::from_bits(datum.value() as _) + } else { + *(datum.cast_mut_ptr() as *const _) + }; + Some(value) } } } From 56d66f5b41e35456dfb858613d53ae6f7974fc68 Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Thu, 12 Sep 2024 23:08:24 +0300 Subject: [PATCH 2/7] Updated readme to reflect the changes --- README.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 4d00ca994e..836b4f39d2 100644 --- a/README.md +++ b/README.md @@ -84,11 +84,9 @@ those remain untested. So far, some of PGRX's build tooling works on Windows, bu ‡ A local PostgreSQL server installation is not required. `cargo pgrx` can download and compile PostgreSQL versions on its own. - ⹋ PGRX has not been tested to work on 32-bit: the library assumes an 8-byte `pg_sys::Datum` -which may result in unexpected behavior on 32-bit, like dropping 4 bytes of data from `int8` -and `double`. This may not be "unsound" in itself, as it is "merely" illogical, -but it may undermine otherwise-reasonable safety assumptions of PGRX extensions. -We do not plan to add support without considerable ongoing technical and financial contributions. + ⹋ PGRX has not been tested to work on 32-bit, but the library correcly handles conversion of `pg_sys::Datum` +to and from `int8` and `double` types. Use it only for your own risk. We do not plan to add offical support +without considerable ongoing technical and financial contributions.
How to: Homebrew on macOS From ab80959a169fc57cd4af5da2cd2d9cf1361460aa Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Sat, 14 Sep 2024 22:19:25 +0300 Subject: [PATCH 3/7] Reworded the 32-bit compatibility paragraph --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 836b4f39d2..10808a6096 100644 --- a/README.md +++ b/README.md @@ -84,7 +84,7 @@ those remain untested. So far, some of PGRX's build tooling works on Windows, bu ‡ A local PostgreSQL server installation is not required. `cargo pgrx` can download and compile PostgreSQL versions on its own. - ⹋ PGRX has not been tested to work on 32-bit, but the library correcly handles conversion of `pg_sys::Datum` + ⹋ PGRX has not been tested to work on 32-bit, but the library attempts to handle conversion of `pg_sys::Datum` to and from `int8` and `double` types. Use it only for your own risk. We do not plan to add offical support without considerable ongoing technical and financial contributions. From 4c85a414b1ff40568f3ccd68599c50ff26f9e889 Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Mon, 16 Sep 2024 16:43:20 +0300 Subject: [PATCH 4/7] Used size_of:: --- pgrx-pg-sys/src/submodules/datum.rs | 4 ++-- pgrx/src/datum/from.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pgrx-pg-sys/src/submodules/datum.rs b/pgrx-pg-sys/src/submodules/datum.rs index 5252d9a1df..846c2b9ca6 100644 --- a/pgrx-pg-sys/src/submodules/datum.rs +++ b/pgrx-pg-sys/src/submodules/datum.rs @@ -129,7 +129,7 @@ impl From for Datum { impl From for Datum { #[inline] fn from(val: u64) -> Datum { - if size_of::() <= size_of::() { + if size_of::() <= size_of::() { Datum::from(val as usize) } else { Datum::from(Box::into_raw(Box::new(val))) @@ -161,7 +161,7 @@ impl From for Datum { impl From for Datum { #[inline] fn from(val: i64) -> Datum { - if size_of::() <= size_of::() { + if size_of::() <= size_of::() { Datum::from(val as usize) } else { Datum::from(Box::into_raw(Box::new(val))) diff --git a/pgrx/src/datum/from.rs b/pgrx/src/datum/from.rs index 171f28f918..9f67a4b137 100644 --- a/pgrx/src/datum/from.rs +++ b/pgrx/src/datum/from.rs @@ -283,7 +283,7 @@ impl FromDatum for i64 { if is_null { None } else { - let value = if size_of::() <= size_of::() { + let value = if size_of::() <= size_of::() { datum.value() as _ } else { *(datum.cast_mut_ptr() as *const _) @@ -320,7 +320,7 @@ impl FromDatum for f64 { if is_null { None } else { - let value = if size_of::() <= size_of::() { + let value = if size_of::() <= size_of::() { f64::from_bits(datum.value() as _) } else { *(datum.cast_mut_ptr() as *const _) From ce212ce2450b5680c2cf11bddc77e033c841e87e Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Mon, 16 Sep 2024 17:30:29 +0300 Subject: [PATCH 5/7] Used palloc instead of Box --- pgrx-pg-sys/src/submodules/datum.rs | 12 ++++++++++-- pgrx/src/datum/from.rs | 4 ++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/pgrx-pg-sys/src/submodules/datum.rs b/pgrx-pg-sys/src/submodules/datum.rs index 846c2b9ca6..df288b68ca 100644 --- a/pgrx-pg-sys/src/submodules/datum.rs +++ b/pgrx-pg-sys/src/submodules/datum.rs @@ -132,7 +132,11 @@ impl From for Datum { if size_of::() <= size_of::() { Datum::from(val as usize) } else { - Datum::from(Box::into_raw(Box::new(val))) + unsafe { + let ptr = crate::palloc(size_of::()) as *mut u64; + *ptr = val; + Datum::from(ptr) + } } } } @@ -164,7 +168,11 @@ impl From for Datum { if size_of::() <= size_of::() { Datum::from(val as usize) } else { - Datum::from(Box::into_raw(Box::new(val))) + unsafe { + let ptr = crate::palloc(size_of::()) as *mut i64; + *ptr = val; + Datum::from(ptr) + } } } } diff --git a/pgrx/src/datum/from.rs b/pgrx/src/datum/from.rs index 9f67a4b137..0194430681 100644 --- a/pgrx/src/datum/from.rs +++ b/pgrx/src/datum/from.rs @@ -283,7 +283,7 @@ impl FromDatum for i64 { if is_null { None } else { - let value = if size_of::() <= size_of::() { + let value = if size_of::() <= size_of::() { datum.value() as _ } else { *(datum.cast_mut_ptr() as *const _) @@ -320,7 +320,7 @@ impl FromDatum for f64 { if is_null { None } else { - let value = if size_of::() <= size_of::() { + let value = if size_of::() <= size_of::() { f64::from_bits(datum.value() as _) } else { *(datum.cast_mut_ptr() as *const _) From f2657f9f40d27355e30f365a29f2d1dd41babc8b Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Mon, 16 Sep 2024 17:31:54 +0300 Subject: [PATCH 6/7] Changed integer tests to check truncation --- pgrx-pg-sys/src/submodules/datum.rs | 33 +++++++++-------------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/pgrx-pg-sys/src/submodules/datum.rs b/pgrx-pg-sys/src/submodules/datum.rs index df288b68ca..b0d972aec6 100644 --- a/pgrx-pg-sys/src/submodules/datum.rs +++ b/pgrx-pg-sys/src/submodules/datum.rs @@ -239,41 +239,28 @@ mod test { #[test] fn roundtrip_integers() { - #[cfg(target_pointer_width = "64")] - mod types { - pub type UnsignedInt = u64; - pub type SignedInt = i64; - } - #[cfg(target_pointer_width = "32")] - mod types { - // 64-bit integers would be truncated on 32 bit platforms - pub type UnsignedInt = u32; - pub type SignedInt = i32; - } - use types::*; - - let val: SignedInt = 123456; + let val = i64::MAX; let datum = Datum::from(val); - assert_eq!(datum.value() as SignedInt, val); + assert_eq!(datum.value() as i64, val); - let val: isize = 123456; + let val = isize::MAX; let datum = Datum::from(val); assert_eq!(datum.value() as isize, val); - let val: SignedInt = -123456; + let val = i64::MIN; let datum = Datum::from(val); - assert_eq!(datum.value() as SignedInt, val); + assert_eq!(datum.value() as i64, val); - let val: isize = -123456; + let val = isize::MIN; let datum = Datum::from(val); assert_eq!(datum.value() as isize, val); - let val: UnsignedInt = 123456; + let val = u64::MAX; let datum = Datum::from(val); - assert_eq!(datum.value() as UnsignedInt, val); + assert_eq!(datum.value() as u64, val); - let val: usize = 123456; + let val = usize::MAX; let datum = Datum::from(val); - assert_eq!({ datum.value() }, val); + assert_eq!(datum.value(), val); } } From cf1fafb8ae6851d463ec4927b3925901abb84c8d Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Tue, 17 Sep 2024 12:37:45 +0300 Subject: [PATCH 7/7] Used cfg to workaround test issues --- pgrx-pg-sys/src/submodules/datum.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pgrx-pg-sys/src/submodules/datum.rs b/pgrx-pg-sys/src/submodules/datum.rs index b0d972aec6..9865ef5df2 100644 --- a/pgrx-pg-sys/src/submodules/datum.rs +++ b/pgrx-pg-sys/src/submodules/datum.rs @@ -129,7 +129,7 @@ impl From for Datum { impl From for Datum { #[inline] fn from(val: u64) -> Datum { - if size_of::() <= size_of::() { + if cfg!(target_pointer_width = "64") { Datum::from(val as usize) } else { unsafe { @@ -165,7 +165,7 @@ impl From for Datum { impl From for Datum { #[inline] fn from(val: i64) -> Datum { - if size_of::() <= size_of::() { + if cfg!(target_pointer_width = "64") { Datum::from(val as usize) } else { unsafe {