mbutrovich commented on code in PR #3277:
URL: https://github.com/apache/iceberg-rust/pull/3277#discussion_r4401416413
##########
crates/iceberg/src/arrow/reader/pipeline.rs:
##########
@@ -3795,4 +3812,226 @@ mod tests {
let total_rows: usize = batches.iter().map(|b| b.num_rows()).sum();
assert_eq!(total_rows, 3);
}
+
+ /// A data file must finish opening even when nothing polls the scan, and a
+ /// dropped scan must stop opening files (#3276).
+ mod open_progress {
+ use std::ops::Range;
+
+ use async_trait::async_trait;
+ use bytes::Bytes;
+ use futures::StreamExt;
+ use futures::stream::BoxStream;
+ use serde::{Deserialize, Serialize};
+
+ use super::*;
+ use crate::Result;
+ use crate::arrow::scan_metrics::ScanMetrics;
+ use crate::io::{
+ FileIOBuilder, FileMetadata, FileRead, FileWrite, InputFile,
MemoryStorage, OutputFile,
+ Storage, StorageConfig, StorageFactory,
+ };
+ use crate::scan::ArrowRecordBatchStream;
+
+ const CONCURRENCY: usize = 2;
+ /// Smaller than a footer, so one open takes more than one request.
+ const METADATA_SIZE_HINT: usize = 64;
+ /// Bytes one open reads: the hint, then the footer and its indexes.
+ /// A constant because `write_plain_parquet` always writes the same
file.
Review Comment:
Could this comment say that a `parquet` upgrade can change the number? The
file isn't fixed across versions. Its footer stores `created_by`, which
defaults to `"parquet-rs version "` plus the crate version
([`properties.rs#L52`](https://github.com/apache/arrow-rs/blob/f90e061326bd821a7af09281d9e92de6f3b603d9/parquet/src/file/properties.rs#L52)).
I set `created_by` to `parquet-rs version 100.0.0` in `write_plain_parquet`,
and `test_open_proceeds_without_the_scan_being_polled` failed with `left: 1096,
right: 1094` and "the opens stopped where the scan stopped polling". Whoever
does the next upgrade will read that message as a reader bug, so the comment is
where they'd find the real cause.
```suggestion
/// Bytes one open reads: the hint, then the footer and its indexes.
/// The footer stores the writer's `created_by` version, so a
`parquet`
/// upgrade can change this.
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
