Jefffrey commented on code in PR #11348:
URL: https://github.com/apache/arrow-rs/pull/11348#discussion_r4193266724
##########
parquet/src/arrow/arrow_reader/statistics.rs:
##########
@@ -85,6 +86,32 @@ pub(crate) fn from_bytes_to_f16(b: &[u8]) -> Option<f16> {
}
}
+/// Convert both bounds together: if either wraps in the reader's timestamp
unit,
+/// neither bound can safely describe the decoded values.
+fn int96_statistics(min: &Int96, max: &Int96, unit: &TimeUnit) -> Option<(i64,
i64)> {
Review Comment:
i wonder if theres a way to reuse these existing methods:
https://github.com/apache/arrow-rs/blob/d18ea16f6fc9af0ba7e8ea76f6934c6e6cb57a26/parquet/src/data_type.rs#L77-L119
they have a caveat in that they dont check overflowing, though it seems
since they are already used for parsing into arrow buffers i believe it stays
consistent 🤔
##########
parquet/src/arrow/arrow_reader/statistics.rs:
##########
@@ -474,7 +504,23 @@ macro_rules! get_statistics {
DataType::Date64 if $physical_type == Some(PhysicalType::INT64) =>
Ok(Arc::new(Date64Array::from_iter(
$int64_iter::new($iterator).map(|x| x.copied()),))),
DataType::Timestamp(unit, timezone) =>{
- let iter = $int64_iter::new($iterator).map(|x| x.copied());
+ let iter = $iterator.map(|statistics| {
Review Comment:
i wonder if theres a way to try consistent with the other arms, maybe
something like the decimal arms
(mainly in reference to how we introduce the `true`/`false` boolean amid all
the other iterator structs
--
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]
