viirya commented on code in PR #11276:
URL: https://github.com/apache/arrow-rs/pull/11276#discussion_r4130514010
##########
parquet/src/file/metadata/push_decoder.rs:
##########
@@ -651,6 +675,68 @@ mod tests {
assert!(metadata.page_index().is_none()); // of course, we did not
read the page index
}
+ /// Each setter invalidates pending work, including narrower, wider and
Skip policies.
+ #[test]
+ fn test_metadata_decoder_pending_range_policy_changes() {
+ let metadata = test_metadata_without_indexes();
+ let setters: [fn(ParquetMetaDataPushDecoder, PageIndexPolicy) ->
ParquetMetaDataPushDecoder;
+ 3] = [
+ ParquetMetaDataPushDecoder::with_page_index_policy,
+ ParquetMetaDataPushDecoder::with_column_index_policy,
+ ParquetMetaDataPushDecoder::with_offset_index_policy,
+ ];
+ for (column_policy, offset_policy) in [
+ (PageIndexPolicy::Optional, PageIndexPolicy::Optional),
+ (PageIndexPolicy::Skip, PageIndexPolicy::Optional),
+ (PageIndexPolicy::Optional, PageIndexPolicy::Skip),
+ ] {
+ for setter in setters {
+ for policy in [
+ PageIndexPolicy::Skip,
+ PageIndexPolicy::Optional,
+ PageIndexPolicy::Required,
+ ] {
+ let make_decoder = || {
+ ParquetMetaDataPushDecoder::try_new_with_metadata(
+ test_file_len(),
+ metadata.clone(),
+ )
+ .unwrap()
+ .with_column_index_policy(column_policy)
+ .with_offset_index_policy(offset_policy)
+ };
+ let mut decoder = make_decoder();
+ expect_needs_data(decoder.try_decode());
+ assert!(decoder.pending_page_index_range.is_some());
+ decoder = setter(decoder, policy);
+ assert!(decoder.pending_page_index_range.is_none());
+ let mut reference = setter(make_decoder(), policy);
+ push_ranges_to_metadata_decoder(&mut reference,
vec![test_file_range()]);
+ let expected = expect_data(reference.try_decode());
Review Comment:
Non-blocking: could we also cover an error after a pending request? This
fixture has complete indexes, so the `Required` cases exercise successful
decoding but not enforcement when an offset index is missing.
A useful case would remove one chunk’s offset-index metadata, start with
`Optional`, obtain `NeedsData`, then switch to `Required` and provide the
requested bytes. Assert that decoding reports the missing offset index and
leaves `pending_page_index_range` empty.
A separate corrupt-index-bytes case would cover cache cleanup on a parser
error. Both cases passed in scratch tests; adding them here would protect the
error-path behavior described by the implementation comment.
--
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]
