alamb commented on code in PR #6450:
URL: https://github.com/apache/arrow-rs/pull/6450#discussion_r1776086910
##########
parquet/src/file/metadata/reader.rs:
##########
@@ -247,13 +248,18 @@ impl ParquetMetaDataReader {
));
}
- // TODO(ets): what is the correct behavior for missing page indexes?
MetadataLoader would
- // leave them as `None`, while the parser in
`index_reader::read_columns_indexes` returns a
- // vector of empty vectors.
- // I think it's best to leave them as `None`.
+ // FIXME: there are differing implementations in the case where page
indexes are missing
+ // from the file. `MetadataLoader` will leave them as `None`, while
the parser in
+ // `index_reader::read_columns_indexes` returns a vector of empty
vectors.
+ // It is best for this function to replicate the latter behavior for
now, but in the future
+ // the two paths to retrieve metadata should be made consistent. Note
that this is only
+ // an issue if the user requested page indexes, so there is no need to
provide empty
+ // vectors in `try_parse_sized()`.
Review Comment:
```suggestion
// It is best for this function to replicate the latter behavior for
now, but in a future
// breaking release, the two paths to retrieve metadata should be
made consistent. Note that this is only
// an issue if the user requested page indexes, so there is no need
to provide empty
// vectors in `try_parse_sized()`.
```
--
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]
