etseidl commented on code in PR #11159:
URL: https://github.com/apache/arrow-rs/pull/11159#discussion_r4110240982
##########
parquet/src/file/metadata/page_index.rs:
##########
@@ -575,53 +799,54 @@ impl PageIndexBuilder {
///
/// This can be used to add offset index storage to a builder that lacks
one
/// (either a `Default` builder, or one created from a [`PageIndex`]
without offset indexes).
+ /// This replaces any existing offset index storage and discards its
entries.
+ ///
+ /// # Panics
+ ///
+ /// Panics if either dimension exceeds `u32::MAX`.
pub fn allocate_offset_indexes(&mut self, num_row_groups: usize,
num_columns: usize) {
- self.offset_indexes = Self::empty_index(num_row_groups, num_columns);
+ let keep_cols =
+ Keep::new_full(num_columns).expect("page index column count
exceeds u32::MAX");
+ let keep_rows =
+ Keep::new_full(num_row_groups).expect("page index row group count
exceeds u32::MAX");
+ self.offset_indexes = Some(Grid::new(keep_rows, keep_cols));
}
/// Sets the column index for a specific row group and column
///
- /// If column indexes were not allocated (see
[`Self::allocate_column_indexes`]),
- /// or the row group or column index is out of bounds, this method does
nothing.
+ /// Returns `false`, and drops `column_index`, if column indexes were not
allocated
+ /// (see [`Self::allocate_column_indexes`]) or the grid has no storage for
the position.
pub fn put_column_index(
&mut self,
column_index: ColumnIndexMetaData,
row_group_idx: usize,
column_idx: usize,
- ) {
- if let Some(ref mut indexes) = self.column_indexes
- && let Some(row_group) = indexes.get_mut(row_group_idx)
- && let Some(column_slot) = row_group.get_mut(column_idx)
- {
- *column_slot = Some(column_index);
- }
+ ) -> bool {
Review Comment:
breaking change, this should be reverted (and same for `put_offset_index`)
--
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]
