etseidl commented on code in PR #10784:
URL: https://github.com/apache/arrow-rs/pull/10784#discussion_r3832617874
##########
parquet/src/file/metadata/parser.rs:
##########
@@ -241,108 +241,119 @@ pub(crate) fn decode_metadata(
/// Required, Optional, Skip).
/// * `offset_index_policy` - The policy for handling offset index parsing
(e.g.,
/// Required, Optional, Skip).
-/// * `bytes` - The byte slice containing the column index data.
+/// * `bytes` - The byte slice containing the page index data.
/// * `start_offset` - The offset where `bytes` begin in the file.
pub(crate) fn parse_page_index(
metadata: &mut ParquetMetaData,
- column_index_policy: PageIndexPolicy,
- offset_index_policy: PageIndexPolicy,
+ column_index_policy: &PageIndexPolicy,
+ offset_index_policy: &PageIndexPolicy,
bytes: &Bytes,
start_offset: u64,
) -> crate::errors::Result<()> {
- if column_index_policy == PageIndexPolicy::Skip && offset_index_policy ==
PageIndexPolicy::Skip
+ if *column_index_policy == PageIndexPolicy::Skip
+ && *offset_index_policy == PageIndexPolicy::Skip
{
return Ok(());
}
- let column_indexes = parse_column_index(metadata, column_index_policy,
bytes, start_offset)?;
- let offset_indexes = parse_offset_index(metadata, offset_index_policy,
bytes, start_offset)?;
- // this likely shouldn't happen, but check just in case
- if column_indexes.is_none() && offset_indexes.is_none() {
+ let num_row_groups = metadata.num_row_groups();
+ let num_columns = metadata.file_metadata().schema_descr().num_columns();
+ let mut builder = PageIndexBuilder::new_with_policy(
+ num_row_groups,
+ num_columns,
+ column_index_policy.clone(),
+ offset_index_policy.clone(),
+ );
+ parse_column_index(
+ metadata,
+ column_index_policy,
+ &mut builder,
+ bytes,
+ start_offset,
+ )?;
+ parse_offset_index(
+ metadata,
+ offset_index_policy,
+ &mut builder,
+ bytes,
+ start_offset,
+ )?;
+ let page_index = builder.build();
+ // if both indexes are missing from the file, return without modifying
`metadata`
+ if !page_index.has_column_indexes() && !page_index.has_offset_indexes() {
return Ok(());
}
- let page_index = PageIndex::new(column_indexes, offset_indexes);
metadata.set_page_index(Some(page_index));
Ok(())
}
fn parse_column_index(
metadata: &ParquetMetaData,
- column_index_policy: PageIndexPolicy,
+ column_index_policy: &PageIndexPolicy,
+ page_index_builder: &mut PageIndexBuilder,
bytes: &Bytes,
start_offset: u64,
-) -> crate::errors::Result<Option<Vec<Vec<Option<ColumnIndexMetaData>>>>> {
- if column_index_policy == PageIndexPolicy::Skip {
- return Ok(None);
+) -> crate::errors::Result<()> {
+ if *column_index_policy == PageIndexPolicy::Skip {
+ return Ok(());
+ }
+ for rg_idx in 0..metadata.num_row_groups() {
+ let rg = metadata.row_group(rg_idx);
+ for col_idx in 0..rg.num_columns() {
+ if !column_index_policy.is_keep_column(col_idx) {
Review Comment:
I could add a helper to extract an iter over the required columns and
simplify this loop
--
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]
