From c2cf2177f58af328db993bae36d54a77024e5c8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Horstmann?= Date: Sun, 21 Jun 2026 16:51:04 +0200 Subject: [PATCH 1/4] Split traits for plain and bitpacked decoding --- parquet/src/encodings/rle.rs | 13 +- parquet/src/util/bit_util.rs | 223 ++++++++++++++++------------------- 2 files changed, 104 insertions(+), 132 deletions(-) diff --git a/parquet/src/encodings/rle.rs b/parquet/src/encodings/rle.rs index 47e52d00b094..d38a892943c5 100644 --- a/parquet/src/encodings/rle.rs +++ b/parquet/src/encodings/rle.rs @@ -400,13 +400,10 @@ impl RleDecoder { } let value = if self.rle_left > 0 { - let rle_value = T::try_from_le_slice( - &self - .current_value - .as_mut() - .ok_or_else(|| general_err!("current_value should be Some"))? - .to_ne_bytes(), - )?; + let current_value = self + .current_value + .ok_or_else(|| general_err!("current_value should be Some"))?; + let rle_value = T::from_u64(current_value); self.rle_left -= 1; rle_value } else { @@ -434,7 +431,7 @@ impl RleDecoder { if self.rle_left > 0 { let num_values = cmp::min(buffer.len() - values_read, self.rle_left as usize); let repeated_value = - T::try_from_le_slice(&self.current_value.as_mut().unwrap().to_ne_bytes())?; + T::from_u64(self.current_value.unwrap()); buffer[values_read..values_read + num_values].fill(repeated_value); self.rle_left -= num_values as u32; values_read += num_values; diff --git a/parquet/src/util/bit_util.rs b/parquet/src/util/bit_util.rs index d0b13072dc4e..31589a2d57ad 100644 --- a/parquet/src/util/bit_util.rs +++ b/parquet/src/util/bit_util.rs @@ -36,11 +36,9 @@ fn array_from_slice(bs: &[u8]) -> Result<[u8; N]> { } } -/// # Safety -/// All bit patterns 00000xxxx, where there are `BIT_CAPACITY` `x`s, -/// must be valid, unless BIT_CAPACITY is 0. -pub unsafe trait FromBytes: Sized { - const BIT_CAPACITY: usize; +/// Types that can be decoded from plain representations. This includes non-primitive types like +/// `FixedLenByteArray` and also variable length types like `ByteArray`. +pub trait FromBytes: Sized { type Buffer: AsMut<[u8]> + Default; fn try_from_le_slice(b: &[u8]) -> Result; fn from_le_bytes(bs: Self::Buffer) -> Self; @@ -52,17 +50,27 @@ pub unsafe trait FromBytes: Sized { /// directly converted from a u64 value. Types like Int96, ByteArray, /// and FixedLenByteArray that cannot be represented in 64 bits do not /// implement this trait. -pub trait FromBitpacked: FromBytes { +pub trait FromBitpacked { + /// The maximum number of bits that are allowed to be converted to this type. + /// This is at most the size of the type in bits, but could be less, for example + /// for the boolean type. + const BIT_CAPACITY: usize; + /// How many values are converted by one call to `unpack_batch`. + const BATCH_SIZE: usize; /// Convert directly from a u64 value by truncation, avoiding byte slice copies. fn from_u64(v: u64) -> Self; + + /// Converts multiple bitpacked values from `input` to `output`. + /// The `output` slice needs to have space for at least `BATCH_SIZE` elements, + /// otherwise this method will panic. + fn unpack_batch(input: &[u8], output: &mut [Self], num_bits: usize) where Self: Sized; } macro_rules! from_le_bytes { ($($ty: ty),*) => { $( // SAFETY: this macro is used for types for which all bit patterns are valid. - unsafe impl FromBytes for $ty { - const BIT_CAPACITY: usize = std::mem::size_of::<$ty>() * 8; + impl FromBytes for $ty { type Buffer = [u8; size_of::()]; fn try_from_le_slice(b: &[u8]) -> Result { Ok(Self::from_le_bytes(array_from_slice(b)?)) @@ -71,21 +79,82 @@ macro_rules! from_le_bytes { <$ty>::from_le_bytes(bs) } } - impl FromBitpacked for $ty { - #[inline] - fn from_u64(v: u64) -> Self { - v as Self - } - } )* }; } +macro_rules! from_bitpacked { + ($($ty: ty => $unpack: path),*) => { + $( + impl FromBitpacked for $ty { + const BIT_CAPACITY: usize = std::mem::size_of::<$ty>() * 8; + // this has to match the signature of the unpack* functions + const BATCH_SIZE: usize = std::mem::size_of::<$ty>() * 8; + + #[inline] + fn from_u64(v: u64) -> Self { + v as _ + } + + #[inline] + fn unpack_batch(input: &[u8], output: &mut [Self], num_bits: usize) { + $unpack(input, (&mut output[..Self::BATCH_SIZE]).try_into().unwrap(), num_bits) + } + } + )* + } +} + +macro_rules! from_bitpacked_delegate { + ($($ty: ty => $delegate: ty),*) => { + $( + impl FromBitpacked for $ty { + const BIT_CAPACITY: usize = <$delegate as FromBitpacked>::BIT_CAPACITY; + const BATCH_SIZE: usize = <$delegate as FromBitpacked>::BATCH_SIZE; + + fn from_u64(v: u64) -> Self { + v as _ + } + + fn unpack_batch(input: &[u8], output: &mut [Self], num_bits: usize) { + const { + assert!(std::mem::size_of::<$ty>() == std::mem::size_of::<$delegate>()); + assert!(std::mem::align_of::<$ty>() == std::mem::align_of::<$delegate>()); + } + // Safety: ty and delegate have the same size and alignment, and this macro is only used for types that have transmutable bit patterns. + let output: &mut [$delegate] = unsafe { std::slice::from_raw_parts_mut(output.as_mut_ptr().cast::<$delegate>(), output.len()) }; + <$delegate>::unpack_batch(input, output, num_bits); + } + } + )* + } +} + + from_le_bytes! { u8, u16, u32, u64, i8, i16, i32, i64 } +from_bitpacked!(u8 => unpack8, u16 => unpack16, u32 => unpack32, u64 => unpack64); +from_bitpacked_delegate!(i8 => u8 , i16 => u16, i32 => u32, i64 => u64); + +impl FromBitpacked for bool { + const BIT_CAPACITY: usize = 1; + const BATCH_SIZE: usize = ::BATCH_SIZE; + + fn from_u64(v: u64) -> Self { + v != 0 + } + + fn unpack_batch(input: &[u8], output: &mut [Self], num_bits: usize) + { + assert!(num_bits == 1); + // Safety: + // we asserted that we will only decode with a bitwidth of 1, + // so the u8 can only be 0 or 1, which are the valid representations of a bool. + let output: &mut [u8] = unsafe { std::slice::from_raw_parts_mut(output.as_mut_ptr().cast::(), output.len()) }; + u8::unpack_batch(input, output, num_bits); + } +} -// SAFETY: all bit patterns are valid for f32 and f64. -unsafe impl FromBytes for f32 { - const BIT_CAPACITY: usize = 32; +impl FromBytes for f32 { type Buffer = [u8; 4]; fn try_from_le_slice(b: &[u8]) -> Result { Ok(Self::from_le_bytes(array_from_slice(b)?)) @@ -95,16 +164,7 @@ unsafe impl FromBytes for f32 { } } -impl FromBitpacked for f32 { - #[inline] - fn from_u64(v: u64) -> Self { - f32::from_bits(v as u32) - } -} - -// SAFETY: all bit patterns are valid for f64. -unsafe impl FromBytes for f64 { - const BIT_CAPACITY: usize = 64; +impl FromBytes for f64 { type Buffer = [u8; 8]; fn try_from_le_slice(b: &[u8]) -> Result { Ok(Self::from_le_bytes(array_from_slice(b)?)) @@ -114,16 +174,7 @@ unsafe impl FromBytes for f64 { } } -impl FromBitpacked for f64 { - #[inline] - fn from_u64(v: u64) -> Self { - f64::from_bits(v) - } -} - -// SAFETY: the 0000000x bit pattern is always valid for `bool`. -unsafe impl FromBytes for bool { - const BIT_CAPACITY: usize = 1; +impl FromBytes for bool { type Buffer = [u8; 1]; fn try_from_le_slice(b: &[u8]) -> Result { @@ -134,16 +185,7 @@ unsafe impl FromBytes for bool { } } -impl FromBitpacked for bool { - #[inline] - fn from_u64(v: u64) -> Self { - v != 0 - } -} - -// SAFETY: BIT_CAPACITY is 0. -unsafe impl FromBytes for Int96 { - const BIT_CAPACITY: usize = 0; +impl FromBytes for Int96 { type Buffer = [u8; 12]; fn try_from_le_slice(b: &[u8]) -> Result { @@ -168,9 +210,7 @@ unsafe impl FromBytes for Int96 { } } -// SAFETY: BIT_CAPACITY is 0. -unsafe impl FromBytes for ByteArray { - const BIT_CAPACITY: usize = 0; +impl FromBytes for ByteArray { type Buffer = Vec; fn try_from_le_slice(b: &[u8]) -> Result { @@ -181,9 +221,7 @@ unsafe impl FromBytes for ByteArray { } } -// SAFETY: BIT_CAPACITY is 0. -unsafe impl FromBytes for FixedLenByteArray { - const BIT_CAPACITY: usize = 0; +impl FromBytes for FixedLenByteArray { type Buffer = Vec; fn try_from_le_slice(b: &[u8]) -> Result { @@ -671,64 +709,10 @@ impl BitReader { assert!(num_bits <= T::BIT_CAPACITY); // Read directly into output buffer - match size_of::() { - 1 => { - let ptr = batch.as_mut_ptr() as *mut u8; - // SAFETY: batch is properly aligned and sized. Caller guarantees that all bit patterns - // in which only the lowest T::BIT_CAPACITY bits of T are set are valid, - // unpack{8,16,32,64} only set to non0 the lowest num_bits bits, and we - // checked that num_bits <= T::BIT_CAPACITY. - let out = unsafe { std::slice::from_raw_parts_mut(ptr, batch.len()) }; - while values_to_read - i >= 8 { - let out_slice = (&mut out[i..i + 8]).try_into().unwrap(); - unpack8(&self.buffer[self.byte_offset..], out_slice, num_bits); - self.byte_offset += num_bits; - i += 8; - } - } - 2 => { - let ptr = batch.as_mut_ptr() as *mut u16; - // SAFETY: batch is properly aligned and sized. Caller guarantees that all bit patterns - // in which only the lowest T::BIT_CAPACITY bits of T are set are valid, - // unpack{8,16,32,64} only set to non0 the lowest num_bits bits, and we - // checked that num_bits <= T::BIT_CAPACITY. - let out = unsafe { std::slice::from_raw_parts_mut(ptr, batch.len()) }; - while values_to_read - i >= 16 { - let out_slice = (&mut out[i..i + 16]).try_into().unwrap(); - unpack16(&self.buffer[self.byte_offset..], out_slice, num_bits); - self.byte_offset += 2 * num_bits; - i += 16; - } - } - 4 => { - let ptr = batch.as_mut_ptr() as *mut u32; - // SAFETY: batch is properly aligned and sized. Caller guarantees that all bit patterns - // in which only the lowest T::BIT_CAPACITY bits of T are set are valid, - // unpack{8,16,32,64} only set to non0 the lowest num_bits bits, and we - // checked that num_bits <= T::BIT_CAPACITY. - let out = unsafe { std::slice::from_raw_parts_mut(ptr, batch.len()) }; - while values_to_read - i >= 32 { - let out_slice = (&mut out[i..i + 32]).try_into().unwrap(); - unpack32(&self.buffer[self.byte_offset..], out_slice, num_bits); - self.byte_offset += 4 * num_bits; - i += 32; - } - } - 8 => { - let ptr = batch.as_mut_ptr() as *mut u64; - // SAFETY: batch is properly aligned and sized. Caller guarantees that all bit patterns - // in which only the lowest T::BIT_CAPACITY bits of T are set are valid, - // unpack{8,16,32,64} only set to non0 the lowest num_bits bits, and we - // checked that num_bits <= T::BIT_CAPACITY. - let out = unsafe { std::slice::from_raw_parts_mut(ptr, batch.len()) }; - while values_to_read - i >= 64 { - let out_slice = (&mut out[i..i + 64]).try_into().unwrap(); - unpack64(&self.buffer[self.byte_offset..], out_slice, num_bits); - self.byte_offset += 8 * num_bits; - i += 64; - } - } - _ => unreachable!(), + while values_to_read - i >= T::BATCH_SIZE { + T::unpack_batch(&self.buffer[self.byte_offset..], &mut batch[i..], num_bits); + self.byte_offset += num_bits * T::BATCH_SIZE / 8; + i += T::BATCH_SIZE; } // Try to read smaller batches if possible @@ -738,10 +722,7 @@ impl BitReader { self.byte_offset += 4 * num_bits; for out in out_buf { - // Zero-allocate buffer - let mut out_bytes = T::Buffer::default(); - out_bytes.as_mut()[..4].copy_from_slice(&out.to_le_bytes()); - batch[i] = T::from_le_bytes(out_bytes); + batch[i] = T::from_u64(out as u64); i += 1; } } @@ -752,10 +733,7 @@ impl BitReader { self.byte_offset += 2 * num_bits; for out in out_buf { - // Zero-allocate buffer - let mut out_bytes = T::Buffer::default(); - out_bytes.as_mut()[..2].copy_from_slice(&out.to_le_bytes()); - batch[i] = T::from_le_bytes(out_bytes); + batch[i] = T::from_u64(out as u64); i += 1; } } @@ -766,10 +744,7 @@ impl BitReader { self.byte_offset += num_bits; for out in out_buf { - // Zero-allocate buffer - let mut out_bytes = T::Buffer::default(); - out_bytes.as_mut()[..1].copy_from_slice(&out.to_le_bytes()); - batch[i] = T::from_le_bytes(out_bytes); + batch[i] = T::from_u64(out as u64); i += 1; } } @@ -1254,7 +1229,7 @@ mod tests { // Generic values used to check against actual values read from `get_batch`. let expected_values: Vec = values .iter() - .map(|v| T::try_from_le_slice(v.as_bytes()).unwrap()) + .map(|v| T::from_u64(*v)) .collect(); (0..total).for_each(|i| writer.put_value(values[i], num_bits)); From 8eb015bd95ac93b38a75541f2921cc0d68de7b01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Horstmann?= Date: Sun, 21 Jun 2026 17:23:34 +0200 Subject: [PATCH 2/4] Formatting --- parquet/src/encodings/rle.rs | 3 +-- parquet/src/util/bit_util.rs | 17 ++++++++--------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/parquet/src/encodings/rle.rs b/parquet/src/encodings/rle.rs index d38a892943c5..5de9dc850a34 100644 --- a/parquet/src/encodings/rle.rs +++ b/parquet/src/encodings/rle.rs @@ -430,8 +430,7 @@ impl RleDecoder { while values_read < buffer.len() { if self.rle_left > 0 { let num_values = cmp::min(buffer.len() - values_read, self.rle_left as usize); - let repeated_value = - T::from_u64(self.current_value.unwrap()); + let repeated_value = T::from_u64(self.current_value.unwrap()); buffer[values_read..values_read + num_values].fill(repeated_value); self.rle_left -= num_values as u32; values_read += num_values; diff --git a/parquet/src/util/bit_util.rs b/parquet/src/util/bit_util.rs index 31589a2d57ad..22d0f360369d 100644 --- a/parquet/src/util/bit_util.rs +++ b/parquet/src/util/bit_util.rs @@ -63,7 +63,9 @@ pub trait FromBitpacked { /// Converts multiple bitpacked values from `input` to `output`. /// The `output` slice needs to have space for at least `BATCH_SIZE` elements, /// otherwise this method will panic. - fn unpack_batch(input: &[u8], output: &mut [Self], num_bits: usize) where Self: Sized; + fn unpack_batch(input: &[u8], output: &mut [Self], num_bits: usize) + where + Self: Sized; } macro_rules! from_le_bytes { @@ -130,7 +132,6 @@ macro_rules! from_bitpacked_delegate { } } - from_le_bytes! { u8, u16, u32, u64, i8, i16, i32, i64 } from_bitpacked!(u8 => unpack8, u16 => unpack16, u32 => unpack32, u64 => unpack64); from_bitpacked_delegate!(i8 => u8 , i16 => u16, i32 => u32, i64 => u64); @@ -143,13 +144,14 @@ impl FromBitpacked for bool { v != 0 } - fn unpack_batch(input: &[u8], output: &mut [Self], num_bits: usize) - { + fn unpack_batch(input: &[u8], output: &mut [Self], num_bits: usize) { assert!(num_bits == 1); // Safety: // we asserted that we will only decode with a bitwidth of 1, // so the u8 can only be 0 or 1, which are the valid representations of a bool. - let output: &mut [u8] = unsafe { std::slice::from_raw_parts_mut(output.as_mut_ptr().cast::(), output.len()) }; + let output: &mut [u8] = unsafe { + std::slice::from_raw_parts_mut(output.as_mut_ptr().cast::(), output.len()) + }; u8::unpack_batch(input, output, num_bits); } } @@ -1227,10 +1229,7 @@ mod tests { .collect(); // Generic values used to check against actual values read from `get_batch`. - let expected_values: Vec = values - .iter() - .map(|v| T::from_u64(*v)) - .collect(); + let expected_values: Vec = values.iter().map(|v| T::from_u64(*v)).collect(); (0..total).for_each(|i| writer.put_value(values[i], num_bits)); From a4e27c0e37d095273d03c34902e76d2dc59d91d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Horstmann?= Date: Mon, 22 Jun 2026 23:46:15 +0200 Subject: [PATCH 3/4] Add comments and more inline attributes --- parquet/src/util/bit_util.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/parquet/src/util/bit_util.rs b/parquet/src/util/bit_util.rs index 22d0f360369d..0ad29a5766cc 100644 --- a/parquet/src/util/bit_util.rs +++ b/parquet/src/util/bit_util.rs @@ -114,14 +114,21 @@ macro_rules! from_bitpacked_delegate { const BIT_CAPACITY: usize = <$delegate as FromBitpacked>::BIT_CAPACITY; const BATCH_SIZE: usize = <$delegate as FromBitpacked>::BATCH_SIZE; + #[inline] fn from_u64(v: u64) -> Self { v as _ } + #[inline] fn unpack_batch(input: &[u8], output: &mut [Self], num_bits: usize) { + // Guard against misusages of this macro, due to the const block this will fail + // already at compile-time if the types are not compatible. const { - assert!(std::mem::size_of::<$ty>() == std::mem::size_of::<$delegate>()); - assert!(std::mem::align_of::<$ty>() == std::mem::align_of::<$delegate>()); + assert!( + std::mem::size_of::<$ty>() == std::mem::size_of::<$delegate>() + && std::mem::align_of::<$ty>() == std::mem::align_of::<$delegate>(), + "types need to have the same size and alignment" + ); } // Safety: ty and delegate have the same size and alignment, and this macro is only used for types that have transmutable bit patterns. let output: &mut [$delegate] = unsafe { std::slice::from_raw_parts_mut(output.as_mut_ptr().cast::<$delegate>(), output.len()) }; @@ -134,16 +141,18 @@ macro_rules! from_bitpacked_delegate { from_le_bytes! { u8, u16, u32, u64, i8, i16, i32, i64 } from_bitpacked!(u8 => unpack8, u16 => unpack16, u32 => unpack32, u64 => unpack64); -from_bitpacked_delegate!(i8 => u8 , i16 => u16, i32 => u32, i64 => u64); +from_bitpacked_delegate!(i8 => u8, i16 => u16, i32 => u32, i64 => u64); impl FromBitpacked for bool { const BIT_CAPACITY: usize = 1; const BATCH_SIZE: usize = ::BATCH_SIZE; + #[inline] fn from_u64(v: u64) -> Self { v != 0 } + #[inline] fn unpack_batch(input: &[u8], output: &mut [Self], num_bits: usize) { assert!(num_bits == 1); // Safety: From f90cb45aec5025064f5893153f679e08a54d9f37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Horstmann?= Date: Tue, 23 Jun 2026 09:12:08 +0200 Subject: [PATCH 4/4] Remove no longer needed safety comment Co-authored-by: Jeffrey Vo --- parquet/src/util/bit_util.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/parquet/src/util/bit_util.rs b/parquet/src/util/bit_util.rs index 0ad29a5766cc..22c126261b53 100644 --- a/parquet/src/util/bit_util.rs +++ b/parquet/src/util/bit_util.rs @@ -71,7 +71,6 @@ pub trait FromBitpacked { macro_rules! from_le_bytes { ($($ty: ty),*) => { $( - // SAFETY: this macro is used for types for which all bit patterns are valid. impl FromBytes for $ty { type Buffer = [u8; size_of::()]; fn try_from_le_slice(b: &[u8]) -> Result {