tustvold commented on code in PR #3578:
URL: https://github.com/apache/arrow-rs/pull/3578#discussion_r1082605700
##########
parquet/src/bin/parquet-index.rs:
##########
@@ -132,7 +132,7 @@ fn compute_row_counts(offset_index: &[PageLocation], rows:
i64) -> Vec<i64> {
}
/// Prints index information for a single column chunk
-fn print_index<T: std::fmt::Debug>(
+fn print_index<T: std::fmt::Display>(
Review Comment:
Relates to #3573 FYI @bmmeijers
The indexes now all implement Display :tada:
##########
parquet/src/util/bit_util.rs:
##########
@@ -17,34 +17,34 @@
use std::{cmp, mem::size_of};
-use crate::data_type::AsBytes;
+use crate::data_type::{AsBytes, ByteArray, FixedLenByteArray, Int96};
+use crate::errors::{ParquetError, Result};
use crate::util::bit_pack::{unpack16, unpack32, unpack64, unpack8};
use crate::util::memory::ByteBufferPtr;
#[inline]
pub fn from_ne_slice<T: FromBytes>(bs: &[u8]) -> T {
- let mut b = T::Buffer::default();
- {
- let b = b.as_mut();
- let bs = &bs[..b.len()];
- b.copy_from_slice(bs);
- }
Review Comment:
This logic was problematic as for `ByteArray` and `FixedLenByteArray` use
`Vec` as `Self::Buffer`, this would then have a length of `0` and so wouldn't
copy anything across...
##########
parquet/src/data_type.rs:
##########
@@ -1226,60 +1223,6 @@ impl AsRef<[u8]> for FixedLenByteArray {
}
}
-impl FromBytes for Int96 {
- type Buffer = [u8; 12];
- fn from_le_bytes(bs: Self::Buffer) -> Self {
- let mut i = Int96::new();
- i.set_data(
- from_le_slice(&bs[0..4]),
- from_le_slice(&bs[4..8]),
- from_le_slice(&bs[8..12]),
- );
- i
- }
- fn from_be_bytes(_bs: Self::Buffer) -> Self {
- unimplemented!()
- }
- fn from_ne_bytes(bs: Self::Buffer) -> Self {
- let mut i = Int96::new();
- i.set_data(
- from_ne_slice(&bs[0..4]),
- from_ne_slice(&bs[4..8]),
- from_ne_slice(&bs[8..12]),
- );
- i
- }
-}
-
-// FIXME Needed to satisfy the constraint of many decoding functions but
ByteArray does not
-// appear to actual be converted directly from bytes
-impl FromBytes for ByteArray {
- type Buffer = Vec<u8>;
- fn from_le_bytes(bs: Self::Buffer) -> Self {
- ByteArray::from(bs)
- }
- fn from_be_bytes(_bs: Self::Buffer) -> Self {
- unreachable!()
- }
- fn from_ne_bytes(bs: Self::Buffer) -> Self {
- ByteArray::from(bs)
- }
-}
-
-impl FromBytes for FixedLenByteArray {
- type Buffer = Vec<u8>;
-
- fn from_le_bytes(bs: Self::Buffer) -> Self {
- Self(ByteArray::from(bs))
- }
- fn from_be_bytes(_bs: Self::Buffer) -> Self {
- unreachable!()
- }
- fn from_ne_bytes(bs: Self::Buffer) -> Self {
- Self(ByteArray::from(bs))
- }
-}
Review Comment:
These have been moved to live alongside the other FromBytes implementations
##########
parquet/src/file/serialized_reader.rs:
##########
@@ -1575,20 +1576,6 @@ mod tests {
});
}
- fn check_bytes_page_index(
Review Comment:
We no longer need to treat different index types differently :tada:
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
