Skip to content

Commit

Permalink
Fixed conversion of i64 and f64 to datums on 32-bit machines (#1859)
Browse files Browse the repository at this point in the history
No tests in CI at the moment because GitHub Actions are .NET based
which: actions/runner/issues/1181. May spend some time later on that,
but for now let's keep the warning about official support of x64 only.
  • Loading branch information
YohDeadfall authored Sep 18, 2024
1 parent c2bc604 commit f4a1036
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 33 deletions.
8 changes: 3 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 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.

<details style="border: 1px solid; padding: 0.25em 0.5em 0;">
<summary><i>How to:</i> <b>Homebrew on macOS</b></summary>
Expand Down
53 changes: 28 additions & 25 deletions pgrx-pg-sys/src/submodules/datum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,15 @@ impl From<u32> for Datum {
impl From<u64> for Datum {
#[inline]
fn from(val: u64) -> Datum {
Datum::from(val as usize)
if cfg!(target_pointer_width = "64") {
Datum::from(val as usize)
} else {
unsafe {
let ptr = crate::palloc(size_of::<u64>()) as *mut u64;
*ptr = val;
Datum::from(ptr)
}
}
}
}

Expand Down Expand Up @@ -157,7 +165,15 @@ impl From<i32> for Datum {
impl From<i64> for Datum {
#[inline]
fn from(val: i64) -> Datum {
Datum::from(val as usize)
if cfg!(target_pointer_width = "64") {
Datum::from(val as usize)
} else {
unsafe {
let ptr = crate::palloc(size_of::<i64>()) as *mut i64;
*ptr = val;
Datum::from(ptr)
}
}
}
}

Expand Down Expand Up @@ -223,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);
}
}
16 changes: 13 additions & 3 deletions pgrx/src/datum/from.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -283,7 +283,12 @@ impl FromDatum for i64 {
if is_null {
None
} else {
Some(datum.value() as _)
let value = if size_of::<i64>() <= size_of::<pg_sys::Datum>() {
datum.value() as _
} else {
*(datum.cast_mut_ptr() as *const _)
};
Some(value)
}
}
}
Expand Down Expand Up @@ -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::<i64>() <= size_of::<pg_sys::Datum>() {
f64::from_bits(datum.value() as _)
} else {
*(datum.cast_mut_ptr() as *const _)
};
Some(value)
}
}
}
Expand Down

0 comments on commit f4a1036

Please sign in to comment.