Skip to content

Commit

Permalink
update public api Statistics::max to return an option.
Browse files Browse the repository at this point in the history
I first re-named the existing method to `max_unchecked` and made it
internal to the crate.

I then added a `pub max(&self) -> Opiton<&T>` method.

I figure we can first change the public API before deciding what to do
about internal usage.

Ref: #6093
  • Loading branch information
Michael-J-Ward committed Aug 9, 2024
1 parent fd2ac15 commit 18b467b
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 57 deletions.
14 changes: 7 additions & 7 deletions parquet/src/arrow/arrow_reader/statistics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,18 +137,18 @@ make_stats_iterator!(
);
make_stats_iterator!(
MaxBooleanStatsIterator,
max,
max_unchecked,
ParquetStatistics::Boolean,
bool
);
make_stats_iterator!(MinInt32StatsIterator, min_unchecked, ParquetStatistics::Int32, i32);
make_stats_iterator!(MaxInt32StatsIterator, max, ParquetStatistics::Int32, i32);
make_stats_iterator!(MaxInt32StatsIterator, max_unchecked, ParquetStatistics::Int32, i32);
make_stats_iterator!(MinInt64StatsIterator, min_unchecked, ParquetStatistics::Int64, i64);
make_stats_iterator!(MaxInt64StatsIterator, max, ParquetStatistics::Int64, i64);
make_stats_iterator!(MaxInt64StatsIterator, max_unchecked, ParquetStatistics::Int64, i64);
make_stats_iterator!(MinFloatStatsIterator, min_unchecked, ParquetStatistics::Float, f32);
make_stats_iterator!(MaxFloatStatsIterator, max, ParquetStatistics::Float, f32);
make_stats_iterator!(MaxFloatStatsIterator, max_unchecked, ParquetStatistics::Float, f32);
make_stats_iterator!(MinDoubleStatsIterator, min_unchecked, ParquetStatistics::Double, f64);
make_stats_iterator!(MaxDoubleStatsIterator, max, ParquetStatistics::Double, f64);
make_stats_iterator!(MaxDoubleStatsIterator, max_unchecked, ParquetStatistics::Double, f64);
make_stats_iterator!(
MinByteArrayStatsIterator,
min_bytes,
Expand Down Expand Up @@ -251,7 +251,7 @@ make_decimal_stats_iterator!(
);
make_decimal_stats_iterator!(
MaxDecimal128StatsIterator,
max,
max_unchecked,
max_bytes,
i128,
from_bytes_to_i128
Expand All @@ -265,7 +265,7 @@ make_decimal_stats_iterator!(
);
make_decimal_stats_iterator!(
MaxDecimal256StatsIterator,
max,
max_unchecked,
max_bytes,
i256,
from_bytes_to_i256
Expand Down
8 changes: 4 additions & 4 deletions parquet/src/arrow/arrow_writer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2544,7 +2544,7 @@ mod tests {
assert!(stats.has_min_max_set());
if let Statistics::Int32(stats) = stats {
assert_eq!(*stats.min_unchecked() as u32, *src_slice.iter().min().unwrap());
assert_eq!(*stats.max() as u32, *src_slice.iter().max().unwrap());
assert_eq!(*stats.max_unchecked() as u32, *src_slice.iter().max().unwrap());
} else {
panic!("Statistics::Int32 missing")
}
Expand Down Expand Up @@ -2585,7 +2585,7 @@ mod tests {
assert!(stats.has_min_max_set());
if let Statistics::Int64(stats) = stats {
assert_eq!(*stats.min_unchecked() as u64, *src_slice.iter().min().unwrap());
assert_eq!(*stats.max() as u64, *src_slice.iter().max().unwrap());
assert_eq!(*stats.max_unchecked() as u64, *src_slice.iter().max().unwrap());
} else {
panic!("Statistics::Int64 missing")
}
Expand Down Expand Up @@ -3070,7 +3070,7 @@ mod tests {
// Column chunk of column "a" should have chunk level statistics
if let Statistics::ByteArray(byte_array_stats) = a_col.statistics().unwrap() {
let min = byte_array_stats.min_unchecked();
let max = byte_array_stats.max();
let max = byte_array_stats.max_unchecked();

assert_eq!(min.as_bytes(), &[b'a']);
assert_eq!(max.as_bytes(), &[b'd']);
Expand Down Expand Up @@ -3142,7 +3142,7 @@ mod tests {
// Column chunk of column "a" should have chunk level statistics
if let Statistics::ByteArray(byte_array_stats) = a_col.statistics().unwrap() {
let min = byte_array_stats.min_unchecked();
let max = byte_array_stats.max();
let max = byte_array_stats.max_unchecked();

assert_eq!(min.as_bytes(), &[b'a']);
assert_eq!(max.as_bytes(), &[b'd']);
Expand Down
Loading

0 comments on commit 18b467b

Please sign in to comment.