Skip to content

Commit

Permalink
fix: correct broken serialization
Browse files Browse the repository at this point in the history
Unfortunately the usage of `Internal` in function APIs throws type
safety out of the window. A handful of aggregate serialization
functions were incorrectly implemented, and operated on the incorrect
type, resulting in incorrect data in the serialized type.

Fortunately, this serialization is only used when [partial aggregation]
kicks in, so is not always triggered.

[partial aggregation]: https://www.postgresql.org/docs/current/xaggr.html#XAGGR-PARTIAL-AGGREGATES
  • Loading branch information
JamesGuthrie committed Nov 13, 2024
1 parent 1fa65a0 commit 36aec42
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 7 deletions.
12 changes: 7 additions & 5 deletions extension/src/stats_agg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ impl<'input> StatsSummary2D<'input> {

#[pg_extern(immutable, parallel_safe, strict)]
pub fn stats1d_trans_serialize(state: Internal) -> bytea {
let ser: &StatsSummary1DData = unsafe { state.get().unwrap() };
let ser: &StatsSummary1D = unsafe { state.get().unwrap() };
let ser: &StatsSummary1DData = &ser.0;
crate::do_serialize!(ser)
}

Expand All @@ -125,7 +126,8 @@ pub fn stats1d_trans_deserialize_inner(bytes: bytea) -> Inner<StatsSummary1D<'st

#[pg_extern(immutable, parallel_safe, strict)]
pub fn stats2d_trans_serialize(state: Internal) -> bytea {
let ser: &StatsSummary2DData = unsafe { state.get().unwrap() };
let ser: &StatsSummary2D = unsafe { state.get().unwrap() };
let ser: &StatsSummary2DData = &ser.0;
crate::do_serialize!(ser)
}

Expand Down Expand Up @@ -1654,9 +1656,9 @@ mod tests {
let state = stats1d_trans_inner(state, Some(39.42), ptr::null_mut());
let state = stats1d_trans_inner(state, Some(-43.0), ptr::null_mut());

let control = state.unwrap();
let control = (*state.unwrap()).clone();
let buffer = stats1d_trans_serialize(Inner::from(control.clone()).internal().unwrap());
let buffer = pgrx::varlena::varlena_to_byte_slice(buffer.0.cast_mut_ptr());
let buffer = varlena_to_byte_slice(buffer.0.cast_mut_ptr());

let expected = [
1, 1, 1, 5, 0, 0, 0, 0, 0, 0, 0, 144, 194, 245, 40, 92, 143, 73, 64, 100, 180, 142,
Expand All @@ -1669,7 +1671,7 @@ mod tests {
let new_state =
stats1d_trans_deserialize_inner(bytea(pg_sys::Datum::from(expected.as_ptr())));

assert_eq!(&*new_state, &*control);
assert_eq!(*new_state, control);
}
}

Expand Down
4 changes: 2 additions & 2 deletions extension/src/time_vector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,8 @@ pub fn arrow_timevector_unnest<'a>(

#[pg_extern(immutable, parallel_safe, strict)]
pub fn timevector_serialize(state: Internal) -> bytea {
// FIXME: This might duplicate the version and padding bits
let state: &Timevector_TSTZ_F64Data = unsafe { state.get().unwrap() };
let state: &Timevector_TSTZ_F64 = unsafe { state.get().unwrap() };
let state: &Timevector_TSTZ_F64Data = &state.0;
crate::do_serialize!(state)
}

Expand Down

0 comments on commit 36aec42

Please sign in to comment.