alamb opened a new issue, #11030:
URL: https://github.com/apache/arrow-rs/issues/11030

   **Is your feature request related to a problem or challenge? Please describe 
what you are trying to do.**
   
   - Follow on to https://github.com/apache/arrow-rs/pull/10842 from @etseidl
   
   We added the 
[`PageIndexProvider`](https://github.com/apache/arrow-rs/blob/79dbf5a3524cb8077d58eaec21c17f97de4ba4ca/parquet/src/file/metadata/page_index.rs#L139)
 trait so users can supply custom page index representations. However, when a 
`ParquetMetaData` holds a custom `PageIndexProvider` implementation, 
[`ParquetMetaDataWriter::finish`](https://github.com/apache/arrow-rs/blob/79dbf5a3524cb8077d58eaec21c17f97de4ba4ca/parquet/src/file/metadata/writer.rs#L444)
 **silently omits the page indexes** (`ColumnIndex` / `OffsetIndex`) from the 
output.
   
   This means a user who:
   1. Reads parquet metadata using a custom `PageIndexProvider`, and
   2. Writes that metadata back out with `ParquetMetaDataWriter`
   
   gets a file with **no page indexes** — with no error or warning. Readers of 
that file can no longer do page-level pruning, silently degrading query 
performance.
   
   The cause is that the writer serializes the indexes by downcasting to the 
concrete built-in `PageIndex` type, which fails for custom providers:
   
https://github.com/apache/arrow-rs/blob/79dbf5a3524cb8077d58eaec21c17f97de4ba4ca/parquet/src/file/metadata/writer.rs#L464-L474
   
   ```rust
   // Downcast to PageIndex to access raw index structures for serialization
   // TODO: rework the encoder to work with the PageIndexProvider API to
   // remove the need for this cast. Because the page index is a dyn trait,
   // it can't be cloned. An implementation would have to create a new
   // `PageIndex` from the current `PageIndexProvider` and pass that to
   // the encoder.
   if let Some(page_index_arc) = self.metadata.page_index.as_ref()
       && let Some(page_index) = page_index_arc
           .as_any()
           .downcast_ref::<crate::file::metadata::PageIndex>()
   ```
   
   **Describe the solution you'd like**
   
   Rework the metadata thrift encoder to work with the `PageIndexProvider` API 
directly, so page indexes are written regardless of the concrete provider 
implementation.
   
   **Describe alternatives you've considered**
   
   - Materialize a built-in `PageIndex` from any custom provider before 
serialization (simpler, but requires an extra copy of the index data).
   - Keep the limitation but make it visible: document it on 
`ParquetMetaDataWriter` and/or return an error instead of silently omitting the 
indexes.
   
   **Additional context**
   
   - Original review discussion: 
https://github.com/apache/arrow-rs/pull/10842#discussion_r3913829496
   


-- 
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]

Reply via email to