cappyzawa opened a new issue, #3356: URL: https://github.com/apache/iceberg-rust/issues/3356
### Apache Iceberg Rust version main (ecba0b5) ### Describe the bug Scan tasks without predicates or delete files load and decode the Parquet column and offset indexes when those indexes are present, even though the task does not use them. `FileScanTaskReader::process` sets `preload_page_index` to `false` for these tasks: https://github.com/apache/iceberg-rust/blob/ecba0b537a1f78ec8125c1a26f6dfab3f95b509e/crates/iceberg/src/arrow/reader/pipeline.rs#L138-L141 However, `ArrowFileReader::get_metadata` first applies that setting to both page-index policies, then overwrites each policy with `preload_column_index` and `preload_offset_index`. Both default to `true` and have no public setters: https://github.com/apache/iceberg-rust/blob/ecba0b537a1f78ec8125c1a26f6dfab3f95b509e/crates/iceberg/src/arrow/reader/file_reader.rs#L120-L131 Row selection is disabled by default, and `TableScanBuilder::with_row_selection_enabled` notes that parsing page indexes can outweigh the benefit of skipping rows. Tasks without predicates or deletes still decode them. This adds metadata decoding work, and can require an additional range read when the index bytes are not covered by the metadata prefetch. Loading the page index only when a task needs it is the original intent: apache/iceberg-rust#565 introduced the per-task decision and apache/iceberg-rust#950 extended it to delete files, both passing it to `ArrowReaderOptions::with_page_index`. apache/iceberg-rust#1294, a dependency bump, moved the decision into `ArrowFileReader` and set `preload_column_index` / `preload_offset_index` to `true`, after which the offset index was always loaded. apache/iceberg-rust#1728 then changed the setter order so that the column index is always loaded too. This report is limited to tasks without predicates or deletes. Offset indexes can help other paths, since Parquet uses them to fetch only the pages a row selection needs, including selections from positional deletes and from row filters. ### To Reproduce A crate-internal test in `crates/iceberg/src/arrow/reader/file_reader.rs`, since the read options are `pub(crate)`. It writes a 200-column file with 100-row pages and loads its metadata with the options `process` uses for a task without predicates or deletes. The metadata prefetch is disabled so that each range read is visible. <details> <summary>Test</summary> ```rust mod page_index_repro { use std::ops::Range; use std::sync::{Arc, Mutex}; use arrow_array::{ArrayRef, Int32Array, RecordBatch}; use arrow_schema::{DataType, Field, Schema}; use parquet::arrow::ArrowWriter; use parquet::arrow::async_reader::AsyncFileReader; use parquet::file::properties::WriterProperties; use super::super::{ArrowFileReader, ParquetReadOptions}; use crate::io::{FileMetadata, FileRead}; struct RecordingRead { data: bytes::Bytes, reads: Arc<Mutex<Vec<Range<u64>>>>, } #[async_trait::async_trait] impl FileRead for RecordingRead { async fn read(&self, range: Range<u64>) -> crate::Result<bytes::Bytes> { self.reads.lock().unwrap().push(range.clone()); Ok(self.data.slice(range.start as usize..range.end as usize)) } } fn wide_parquet(columns: usize) -> bytes::Bytes { let fields: Vec<Field> = (0..columns) .map(|i| Field::new(format!("c{i}"), DataType::Int32, false)) .collect(); let schema = Arc::new(Schema::new(fields)); let arrays: Vec<ArrayRef> = (0..columns) .map(|_| Arc::new(Int32Array::from_iter_values(0..10_000)) as ArrayRef) .collect(); let batch = RecordBatch::try_new(schema.clone(), arrays).unwrap(); let props = WriterProperties::builder() .set_data_page_row_count_limit(100) .build(); let mut buf = Vec::new(); let mut writer = ArrowWriter::try_new(&mut buf, schema, Some(props)).unwrap(); writer.write(&batch).unwrap(); writer.close().unwrap(); bytes::Bytes::from(buf) } async fn load(options: ParquetReadOptions, data: &bytes::Bytes) { let reads = Arc::new(Mutex::new(vec![])); let mut reader = ArrowFileReader::new( FileMetadata { size: data.len() as u64, }, Box::new(RecordingRead { data: data.clone(), reads: reads.clone(), }), ) .with_parquet_read_options(options); let metadata = reader.get_metadata(None).await.unwrap(); println!( "column_index={} offset_index={} reads={:?}", metadata.column_index().is_some(), metadata.offset_index().is_some(), reads.lock().unwrap(), ); } #[tokio::test] async fn page_indexes_are_loaded_for_tasks_that_do_not_use_them() { let data = wide_parquet(200); println!("file size = {}", data.len()); // `preload_page_index` defaults to false, which is what `process` // sets for a task without predicates or deletes. let task_options = ParquetReadOptions::builder() .with_metadata_size_hint(None) .build(); load(task_options, &data).await; let mut all_off = task_options; all_off.preload_column_index = false; all_off.preload_offset_index = false; load(all_off, &data).await; } } ``` </details> ``` cargo test -p iceberg --lib page_index_repro -- --nocapture ``` ``` file size = 11255714 column_index=true offset_index=true reads=[11255706..11255714, 11218621..11255706, 11167804..11218621] column_index=false offset_index=false reads=[11255706..11255714, 11218621..11255706] ``` The third range (50,817 bytes, against a 37,085-byte footer) is the page index. With the default 512 KiB prefetch, this file's footer and indexes fit in one read, so the cost here would be decoding only; files whose indexes extend beyond the prefetch also pay the extra read. ### Expected behavior Scan tasks without predicates or delete files do not decode unused page indexes, nor issue range reads specifically to load them. Index bytes may still be included incidentally in the metadata prefetch. ### Willingness to contribute I can contribute a fix for this bug independently --- AI assistants (Claude and Codex) helped investigate the reader logic and draft and review this report. I ran the test above at the listed commit, with metadata prefetch disabled, and the output shown is from that run. End-to-end scan performance, and the effect on tasks with predicates or deletes, have not been measured. -- 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]
