etseidl commented on code in PR #11475:
URL: https://github.com/apache/arrow-rs/pull/11475#discussion_r4298037265
##########
parquet/src/file/metadata/page_index.rs:
##########
@@ -506,77 +562,138 @@ impl HeapSize for PageIndex {
/// - Populating column indexes for predicate columns (for page filtering)
/// - Populating offset indexes for projected columns (for direct I/O)
/// - Automatic conversion of empty structures to `None` to save memory
-#[derive(Default)]
+#[derive(Default, Debug)]
pub struct PageIndexBuilder {
- column_indexes: Option<Vec<Vec<Option<ColumnIndexMetaData>>>>,
- offset_indexes: Option<Vec<Vec<Option<OffsetIndexMetaData>>>>,
+ column_indexes: Option<ColumnChunkMap<ColumnIndexMetaData>>,
+ offset_indexes: Option<ColumnChunkMap<OffsetIndexMetaData>>,
}
impl PageIndexBuilder {
- /// Creates an empty index structure with space for the specified number
of row groups and columns
+ /// Creates a new [`PageIndexBuilder`] for the specified number of row
groups and columns.
///
- /// Returns `Some` containing a nested vector structure where all entries
are initialized to `None`.
- /// The outer vector has one entry per row group, and each inner vector
has one entry per column.
+ /// The dimensions are used to validate inserted coordinates. Storage is
allocated only for
+ /// entries populated using
+ /// [`put_column_index`](Self::put_column_index) and
[`put_offset_index`](Self::put_offset_index).
///
- /// # Type Parameters
- /// * `T` - The type of index this is to be, either `ColumnIndexMetaData`
or `OffsetIndexMetaData`
- fn empty_index<T>(num_row_groups: usize, num_columns: usize) ->
Option<Vec<Vec<Option<T>>>> {
- Some(
- (0..num_row_groups)
- .map(|_| {
- let mut idx = Vec::with_capacity(num_columns);
- idx.resize_with(num_columns, || None);
- idx
- })
- .collect(),
- )
+ /// # Panics
+ ///
+ /// Panics if either dimension exceeds `i32::MAX`, the maximum collection
size representable
+ /// by Thrift.
+ ///
+ /// Use [`try_new`](Self::try_new) for a fallible alternative.
Review Comment:
This is the biggest behavior change (along with the changes to the
infallible `allocate_` functions below). This used to take any valid `usize`
dimension, but now panics if a dimension exceeds what's possible for Parquet.
That said, it may have panicked before anyway if it couldn't allocate a huge
matrix.
--
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]
