andygrove commented on code in PR #558:
URL: https://github.com/apache/datafusion-comet/pull/558#discussion_r1638964444
##########
core/src/parquet/read/values.rs:
##########
@@ -438,41 +419,92 @@ impl PlainDictDecoding for BoolType {
}
}
-// Shared implementation for int variants such as Int8 and Int16
macro_rules! make_int_variant_impl {
- ($ty: ident, $native_ty: ty, $type_size: expr) => {
- impl PlainDecoding for $ty {
+ ($dst_type:ty, $copy_fn:ident, $type_width:expr) => {
+ impl PlainDecoding for $dst_type {
fn decode(src: &mut PlainDecoderInner, dst: &mut
ParquetMutableVector, num: usize) {
- let src_data = &src.data;
let dst_slice = dst.value_buffer.as_slice_mut();
- let mut dst_offset = dst.num_values * $type_size;
- for _ in 0..num {
- unsafe {
- copy_nonoverlapping(
- &src_data[src.offset..] as *const [u8] as *const u8
- as *const $native_ty,
- &mut dst_slice[dst_offset] as *mut u8 as *mut
$native_ty,
- 1,
- );
- }
- src.offset += 4; // Parquet stores Int8/Int16 using 4 bytes
- dst_offset += $type_size;
- }
+ let dst_offset = dst.num_values * $type_width;
+ $copy_fn(&src.data[src.offset..], &mut
dst_slice[dst_offset..], num);
+ src.offset += 4 * num; // Parquet stores Int8/Int16 using 4
bytes
}
fn skip(src: &mut PlainDecoderInner, num: usize) {
- let num_bytes = 4 * num; // Parquet stores Int8/Int16 using 4
bytes
- src.offset += num_bytes;
+ src.offset += 4 * num; // Parquet stores Int8/Int16 using 4
bytes
+ }
+ }
+ };
+}
+
+make_int_variant_impl!(Int8Type, copy_i32_to_i8, 1);
+make_int_variant_impl!(Int16Type, copy_i32_to_i16, 2);
+make_int_variant_impl!(Int32To64Type, copy_i32_to_i64, 4);
+
+// unsigned type require double the width and zeroes are written for the
second half
+// perhaps because they are implemented as the next size up signed type?
+make_int_variant_impl!(UInt8Type, copy_i32_to_u8, 2);
+make_int_variant_impl!(UInt16Type, copy_i32_to_u16, 4);
+make_int_variant_impl!(UInt32Type, copy_i32_to_u32, 8);
+
+macro_rules! generate_cast_to_unsigned {
+ ($name: ident, $src_type:ty, $dst_type:ty, $zero_value:expr) => {
+ pub fn $name(src: &[u8], dst: &mut [u8], num: usize) {
+ debug_assert!(
+ src.len() >= num * std::mem::size_of::<$src_type>(),
+ "Source slice is too small"
+ );
+ debug_assert!(
+ dst.len() >= num * std::mem::size_of::<$dst_type>() * 2,
+ "Destination slice is too small"
+ );
+
+ let src_ptr = src.as_ptr() as *const $src_type;
+ let dst_ptr = dst.as_mut_ptr() as *mut $dst_type;
+ unsafe {
+ for i in 0..num {
+ dst_ptr
+ .add(2 * i)
+ .write_unaligned(src_ptr.add(i).read_unaligned() as
$dst_type);
+ // write zeroes
+ dst_ptr.add(2 * i + 1).write_unaligned($zero_value);
+ }
}
Review Comment:
I think that would involve making an extra copy?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
