From a0a7ccd13550cf8e7ba2a619ca3fa10050e257ab Mon Sep 17 00:00:00 2001 From: "alex.bianchi" Date: Thu, 22 Oct 2026 15:06:36 -0400 Subject: [PATCH 01/15] feat(scan): expose portable reader options for planned tasks --- crates/iceberg/public-api.txt | 22 ++++ crates/iceberg/src/arrow/reader/mod.rs | 2 + .../iceberg/src/arrow/reader/scan_options.rs | 116 ++++++++++++++++++ crates/iceberg/src/scan/mod.rs | 116 +++++++++++++++--- 4 files changed, 241 insertions(+), 15 deletions(-) create mode 100644 crates/iceberg/src/arrow/reader/scan_options.rs diff --git a/crates/iceberg/public-api.txt b/crates/iceberg/public-api.txt index 92267f0a99..9b248bc09a 100644 --- a/crates/iceberg/public-api.txt +++ b/crates/iceberg/public-api.txt @@ -104,6 +104,27 @@ pub struct iceberg::arrow::ScanResult impl iceberg::scan::ScanResult pub fn iceberg::scan::ScanResult::metrics(&self) -> &iceberg::scan::ScanMetrics pub fn iceberg::scan::ScanResult::stream(self) -> iceberg::scan::ArrowRecordBatchStream +pub struct iceberg::arrow::TableScanReaderOptions +impl iceberg::arrow::TableScanReaderOptions +pub fn iceberg::arrow::TableScanReaderOptions::apply_to(&self, builder: iceberg::arrow::ArrowReaderBuilder) -> iceberg::arrow::ArrowReaderBuilder +pub fn iceberg::arrow::TableScanReaderOptions::batch_size(&self) -> core::option::Option +pub fn iceberg::arrow::TableScanReaderOptions::bloom_filter_enabled(&self) -> bool +pub fn iceberg::arrow::TableScanReaderOptions::data_file_concurrency_limit(&self) -> usize +pub fn iceberg::arrow::TableScanReaderOptions::row_group_filtering_enabled(&self) -> bool +pub fn iceberg::arrow::TableScanReaderOptions::row_selection_enabled(&self) -> bool +impl core::clone::Clone for iceberg::arrow::TableScanReaderOptions +pub fn iceberg::arrow::TableScanReaderOptions::clone(&self) -> iceberg::arrow::TableScanReaderOptions +impl core::cmp::Eq for iceberg::arrow::TableScanReaderOptions +impl core::cmp::PartialEq for iceberg::arrow::TableScanReaderOptions +pub fn iceberg::arrow::TableScanReaderOptions::eq(&self, other: &iceberg::arrow::TableScanReaderOptions) -> bool +impl core::fmt::Debug for iceberg::arrow::TableScanReaderOptions +pub fn iceberg::arrow::TableScanReaderOptions::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::Copy for iceberg::arrow::TableScanReaderOptions +impl core::marker::StructuralPartialEq for iceberg::arrow::TableScanReaderOptions +impl serde_core::ser::Serialize for iceberg::arrow::TableScanReaderOptions +pub fn iceberg::arrow::TableScanReaderOptions::serialize<__S>(&self, __serializer: __S) -> core::result::Result<<__S as serde_core::ser::Serializer>::Ok, <__S as serde_core::ser::Serializer>::Error> where __S: serde_core::ser::Serializer +impl<'de> serde_core::de::Deserialize<'de> for iceberg::arrow::TableScanReaderOptions +pub fn iceberg::arrow::TableScanReaderOptions::deserialize<__D>(__deserializer: __D) -> core::result::Result::Error> where __D: serde_core::de::Deserializer<'de> pub const iceberg::arrow::DEFAULT_MAP_FIELD_NAME: &str pub const iceberg::arrow::PROJECTED_PARTITION_VALUE_COLUMN: &str pub const iceberg::arrow::UTC_TIME_ZONE: &str @@ -1436,6 +1457,7 @@ pub struct iceberg::scan::TableScan impl iceberg::scan::TableScan pub fn iceberg::scan::TableScan::column_names(&self) -> core::option::Option<&[alloc::string::String]> pub async fn iceberg::scan::TableScan::plan_files(&self) -> iceberg::Result +pub fn iceberg::scan::TableScan::reader_options(&self) -> iceberg::arrow::TableScanReaderOptions pub fn iceberg::scan::TableScan::snapshot(&self) -> core::option::Option<&iceberg::spec::SnapshotRef> pub async fn iceberg::scan::TableScan::to_arrow(&self) -> iceberg::Result impl core::fmt::Debug for iceberg::scan::TableScan diff --git a/crates/iceberg/src/arrow/reader/mod.rs b/crates/iceberg/src/arrow/reader/mod.rs index cda1909846..e779ae025a 100644 --- a/crates/iceberg/src/arrow/reader/mod.rs +++ b/crates/iceberg/src/arrow/reader/mod.rs @@ -42,6 +42,7 @@ mod predicate_visitor; mod projection; mod row_filter; mod row_lineage; +mod scan_options; pub use file_reader::ArrowFileReader; pub(crate) use options::ParquetReadOptions; use predicate_visitor::{CollectFieldIdVisitor, PredicateConverter}; @@ -49,6 +50,7 @@ use projection::{ add_fallback_field_ids_to_arrow_schema, apply_name_mapping_to_arrow_schema, find_leaf_by_field_id, }; +pub use scan_options::TableScanReaderOptions; /// Builder to create ArrowReader pub struct ArrowReaderBuilder { diff --git a/crates/iceberg/src/arrow/reader/scan_options.rs b/crates/iceberg/src/arrow/reader/scan_options.rs new file mode 100644 index 0000000000..a6ea5c7fac --- /dev/null +++ b/crates/iceberg/src/arrow/reader/scan_options.rs @@ -0,0 +1,116 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use serde::{Deserialize, Serialize}; + +use super::ArrowReaderBuilder; + +/// Reader settings captured from a [`TableScan`](crate::scan::TableScan). +/// +/// Contains no I/O handles, runtime, or planning state. Apply these options to a +/// locally constructed builder, then build one reader and clone it across +/// partitions to share its delete-file cache. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub struct TableScanReaderOptions { + pub(crate) batch_size: Option, + pub(crate) data_file_concurrency_limit: usize, + pub(crate) row_group_filtering_enabled: bool, + pub(crate) row_selection_enabled: bool, + pub(crate) bloom_filter_enabled: bool, +} + +impl TableScanReaderOptions { + /// The requested record batch size, or the reader default. + pub fn batch_size(&self) -> Option { + self.batch_size + } + + /// The maximum number of data files read concurrently. + pub fn data_file_concurrency_limit(&self) -> usize { + self.data_file_concurrency_limit + } + + /// Whether row-group filtering is enabled. + pub fn row_group_filtering_enabled(&self) -> bool { + self.row_group_filtering_enabled + } + + /// Whether page-index row selection is enabled. + pub fn row_selection_enabled(&self) -> bool { + self.row_selection_enabled + } + + /// Whether bloom-filter row-group filtering is enabled. + pub fn bloom_filter_enabled(&self) -> bool { + self.bloom_filter_enabled + } + + /// Applies all scan reader settings, preserving the builder's local resources + /// and Parquet I/O tuning. An unset batch size restores the reader default. + pub fn apply_to(&self, mut builder: ArrowReaderBuilder) -> ArrowReaderBuilder { + builder.batch_size = self.batch_size; + builder.concurrency_limit_data_files = self.data_file_concurrency_limit; + builder.row_group_filtering_enabled = self.row_group_filtering_enabled; + builder.row_selection_enabled = self.row_selection_enabled; + builder.bloom_filter_enabled = self.bloom_filter_enabled; + builder + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::io::FileIO; + use crate::test_utils::test_runtime; + + #[test] + fn applies_scan_settings_and_preserves_local_io_tuning() { + for batch_size in [None, Some(17)] { + let options = TableScanReaderOptions { + batch_size, + data_file_concurrency_limit: 3, + row_group_filtering_enabled: false, + row_selection_enabled: true, + bloom_filter_enabled: true, + }; + let builder = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) + .with_batch_size(1024) + .with_metadata_size_hint(4096) + .with_range_coalesce_bytes(8192) + .with_range_fetch_concurrency(2); + let reader = options.apply_to(builder).build(); + assert_eq!( + ( + reader.batch_size, + reader.concurrency_limit_data_files, + reader.row_group_filtering_enabled, + reader.row_selection_enabled, + reader.bloom_filter_enabled + ), + (batch_size, 3, false, true, true), + ); + assert_eq!( + ( + reader.parquet_read_options.metadata_size_hint(), + reader.parquet_read_options.range_coalesce_bytes(), + reader.parquet_read_options.range_fetch_concurrency() + ), + (Some(4096), 8192, 2), + ); + } + } +} diff --git a/crates/iceberg/src/scan/mod.rs b/crates/iceberg/src/scan/mod.rs index 005b5ea577..e7fd208c99 100644 --- a/crates/iceberg/src/scan/mod.rs +++ b/crates/iceberg/src/scan/mod.rs @@ -33,7 +33,7 @@ use futures::stream::BoxStream; use futures::{SinkExt, StreamExt, TryStreamExt}; pub use task::*; -use crate::arrow::ArrowReaderBuilder; +use crate::arrow::{ArrowReaderBuilder, TableScanReaderOptions}; pub use crate::arrow::{ScanMetrics, ScanResult}; use crate::delete_file_index::DeleteFileIndex; use crate::error::invalid_data; @@ -602,23 +602,51 @@ impl TableScan { /// Returns an [`ArrowRecordBatchStream`]. pub async fn to_arrow(&self) -> Result { - let mut arrow_reader_builder = - ArrowReaderBuilder::new(self.file_io.clone(), self.runtime.clone()) - .with_data_file_concurrency_limit(self.concurrency_limit_data_files) - .with_row_group_filtering_enabled(self.row_group_filtering_enabled) - .with_row_selection_enabled(self.row_selection_enabled) - .with_bloom_filter_enabled(self.bloom_filter_enabled); - - if let Some(batch_size) = self.batch_size { - arrow_reader_builder = arrow_reader_builder.with_batch_size(batch_size); - } - - arrow_reader_builder + self.reader_options() + .apply_to(ArrowReaderBuilder::new( + self.file_io.clone(), + self.runtime.clone(), + )) .build() .read(self.plan_files().await?) .map(|result| result.stream()) } + /// Captures reader settings independently of this scan's planning state. + /// + /// Projection, predicates, and delete references are carried by the planned + /// file tasks; manifest-planning concurrency is not a reader setting. + /// + /// ```no_run + /// # use futures::{TryStreamExt, stream}; + /// # use iceberg::{Result, Runtime, arrow::ArrowReaderBuilder, io::FileIO, table::Table}; + /// # async fn read(table: &Table, local_io: FileIO, local_runtime: Runtime) -> Result<()> { + /// let scan = table.scan().with_batch_size(Some(1024)).build()?; + /// let options = scan.reader_options(); + /// let tasks: Vec<_> = scan.plan_files().await?.try_collect().await?; + /// drop(scan); + /// + /// let reader = options + /// .apply_to(ArrowReaderBuilder::new(local_io, local_runtime)) + /// .build(); + /// let batches: Vec<_> = reader + /// .read(Box::pin(stream::iter(tasks.into_iter().map(Ok))))? + /// .stream() + /// .try_collect() + /// .await?; + /// # Ok(()) + /// # } + /// ``` + pub fn reader_options(&self) -> TableScanReaderOptions { + TableScanReaderOptions { + batch_size: self.batch_size, + data_file_concurrency_limit: self.concurrency_limit_data_files, + row_group_filtering_enabled: self.row_group_filtering_enabled, + row_selection_enabled: self.row_selection_enabled, + bloom_filter_enabled: self.bloom_filter_enabled, + } + } + /// Returns a reference to the column names of the table scan. pub fn column_names(&self) -> Option<&[String]> { self.column_names.as_deref() @@ -750,7 +778,7 @@ mod tests { use uuid::Uuid; use super::resolve_field_id; - use crate::arrow::ArrowReaderBuilder; + use crate::arrow::{ArrowReaderBuilder, TableScanReaderOptions}; use crate::expr::{BoundPredicate, Reference}; use crate::io::FileIO; use crate::metadata_columns::{ @@ -771,7 +799,7 @@ mod tests { }; use crate::test_utils::test_runtime; use crate::util::snapshot::snapshot_id_as_of_time; - use crate::{ErrorKind, TableIdent}; + use crate::{ErrorKind, Runtime, TableIdent}; #[tokio::test] async fn test_table_scan_columns() { @@ -1978,6 +2006,64 @@ mod tests { assert_eq!(int64_arr.value(0), 1); } + #[tokio::test] + async fn reader_options_preserve_reads_after_dropping_scan() { + let mut fixture = TableTestFixture::new(); + fixture.setup_multi_row_group_manifest(&[150, 299]).await; + for enabled in [false, true] { + let scan = fixture + .table + .scan() + .select(["y"]) + .with_filter(Reference::new("y").greater_than(Datum::long(1100))) + .with_batch_size(Some(17)) + .with_data_file_concurrency_limit(1) + .with_row_group_filtering_enabled(enabled) + .with_row_selection_enabled(enabled) + .with_bloom_filter_enabled(enabled) + .build() + .unwrap(); + let expected: Vec<_> = scan.to_arrow().await.unwrap().try_collect().await.unwrap(); + let tasks: Vec<_> = scan + .plan_files() + .await + .unwrap() + .try_collect() + .await + .unwrap(); + let options = scan.reader_options(); + let serialized = serde_json::to_value(options).unwrap(); + assert_eq!( + serialized, + serde_json::json!({ + "batch_size": 17, + "data_file_concurrency_limit": 1, + "row_group_filtering_enabled": enabled, + "row_selection_enabled": enabled, + "bloom_filter_enabled": enabled, + }) + ); + let options: TableScanReaderOptions = serde_json::from_value(serialized).unwrap(); + drop(scan); + let reader = options + .apply_to(ArrowReaderBuilder::new( + FileIO::new_with_fs(), + Runtime::try_current().unwrap(), + )) + .build(); + let actual: Vec<_> = reader + .read(Box::pin(stream::iter(tasks.into_iter().map(Ok)))) + .unwrap() + .stream() + .try_collect() + .await + .unwrap(); + assert!(!actual.is_empty()); + assert!(actual.iter().all(|batch| batch.num_rows() <= 17)); + assert_eq!(actual, expected); + } + } + #[tokio::test] async fn test_open_parquet_no_deletions_by_separate_reader() { let mut fixture = TableTestFixture::new(); From bc6b49a59ec445d0a9ade81a6054cf54fc65e686 Mon Sep 17 00:00:00 2001 From: "alex.bianchi" Date: Fri, 23 Oct 2026 15:43:21 -0400 Subject: [PATCH 02/15] refactor(arrow): separate reader options from scan planning --- crates/catalog/s3tables/src/catalog.rs | 2 +- crates/examples/src/oss_backend.rs | 2 +- crates/iceberg/README.md | 24 +- crates/iceberg/public-api.txt | 47 +- crates/iceberg/src/arrow/reader/mod.rs | 21 +- crates/iceberg/src/arrow/reader/options.rs | 152 ++-- .../src/arrow/reader/parquet_options.rs | 84 +++ .../iceberg/src/arrow/reader/scan_options.rs | 116 ---- crates/iceberg/src/lib.rs | 2 +- crates/iceberg/src/scan/mod.rs | 648 +++++++++--------- .../tests/conflict_commit_test.rs | 2 +- .../tests/read_evolved_schema.rs | 6 +- .../tests/read_positional_deletes.rs | 2 +- crates/storage/opendal/README.md | 2 +- 14 files changed, 588 insertions(+), 522 deletions(-) create mode 100644 crates/iceberg/src/arrow/reader/parquet_options.rs delete mode 100644 crates/iceberg/src/arrow/reader/scan_options.rs diff --git a/crates/catalog/s3tables/src/catalog.rs b/crates/catalog/s3tables/src/catalog.rs index 254f20ae32..c96d06abd4 100644 --- a/crates/catalog/s3tables/src/catalog.rs +++ b/crates/catalog/s3tables/src/catalog.rs @@ -1345,7 +1345,7 @@ mod tests { .select_all() .build() .expect("scan to be valid (snapshot exists, schema is OK)") - .to_arrow() + .to_arrow(Default::default()) .await .expect("scan tasks should be OK") .try_collect() diff --git a/crates/examples/src/oss_backend.rs b/crates/examples/src/oss_backend.rs index 2d56cceb7f..5f56e4a301 100644 --- a/crates/examples/src/oss_backend.rs +++ b/crates/examples/src/oss_backend.rs @@ -81,7 +81,7 @@ async fn main() { println!("Table {TABLE_NAME} loaded!"); let scan = table.scan().select_all().build().unwrap(); - let reader = scan.to_arrow().await.unwrap(); + let reader = scan.to_arrow(Default::default()).await.unwrap(); let buf = reader.collect::>().await; println!("Table {TABLE_NAME} has {} batches.", buf.len()); diff --git a/crates/iceberg/README.md b/crates/iceberg/README.md index 08ce82bd2f..3e06db4719 100644 --- a/crates/iceberg/README.md +++ b/crates/iceberg/README.md @@ -56,7 +56,7 @@ async fn main() -> Result<()> { .scan() .select(["name", "id"]) .build()? - .to_arrow() + .to_arrow(Default::default()) .await?; // Consume this stream like arrow record batch stream. @@ -65,6 +65,28 @@ async fn main() -> Result<()> { } ``` +### Reader settings + +Pass reader settings separately from file planning: + +```rust +let options = iceberg::arrow::ArrowReaderOptions::builder() + .with_batch_size(Some(1024)) + .with_row_selection_enabled(true) + .build(); +let stream = table.scan().build()?.to_arrow(options).await?; +``` + +When upgrading, move reader-setting calls from `TableScanBuilder` to +`ArrowReaderOptions::builder()` and pass the result to `to_arrow(options)`. +Use `to_arrow(Default::default())` for default settings. Scan +`with_concurrency_limit()` controls manifest planning only; set +`with_data_file_concurrency_limit(Some(limit))` on the reader options to also +limit data-file reads. An unset data-file limit uses execution-host parallelism. + +Locally constructed readers accept the same options through +`ArrowReaderBuilder::with_options(options)`. + ## Storage Backends For storage backend support (S3, GCS, local filesystem, etc.), use the [`iceberg-storage-opendal`](https://crates.io/crates/iceberg-storage-opendal) crate. See its [README](../storage/opendal/README.md) for available backends and feature flags. diff --git a/crates/iceberg/public-api.txt b/crates/iceberg/public-api.txt index 9b248bc09a..18c63e6ba2 100644 --- a/crates/iceberg/public-api.txt +++ b/crates/iceberg/public-api.txt @@ -79,6 +79,24 @@ pub fn iceberg::arrow::ArrowReaderBuilder::with_range_coalesce_bytes(self, range pub fn iceberg::arrow::ArrowReaderBuilder::with_range_fetch_concurrency(self, range_fetch_concurrency: usize) -> Self pub fn iceberg::arrow::ArrowReaderBuilder::with_row_group_filtering_enabled(self, row_group_filtering_enabled: bool) -> Self pub fn iceberg::arrow::ArrowReaderBuilder::with_row_selection_enabled(self, row_selection_enabled: bool) -> Self +impl iceberg::arrow::ArrowReaderBuilder +pub fn iceberg::arrow::ArrowReaderBuilder::with_options(self, options: iceberg::arrow::ArrowReaderOptions) -> Self +pub struct iceberg::arrow::ArrowReaderOptions +impl core::clone::Clone for iceberg::arrow::ArrowReaderOptions +pub fn iceberg::arrow::ArrowReaderOptions::clone(&self) -> iceberg::arrow::ArrowReaderOptions +impl core::cmp::PartialEq for iceberg::arrow::ArrowReaderOptions +pub fn iceberg::arrow::ArrowReaderOptions::eq(&self, other: &iceberg::arrow::ArrowReaderOptions) -> bool +impl core::default::Default for iceberg::arrow::ArrowReaderOptions +pub fn iceberg::arrow::ArrowReaderOptions::default() -> Self +impl core::fmt::Debug for iceberg::arrow::ArrowReaderOptions +pub fn iceberg::arrow::ArrowReaderOptions::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for iceberg::arrow::ArrowReaderOptions +impl iceberg::arrow::ArrowReaderOptions +pub fn iceberg::arrow::ArrowReaderOptions::builder() -> ArrowReaderOptionsBuilder<((), (), (), (), ())> +impl serde_core::ser::Serialize for iceberg::arrow::ArrowReaderOptions +pub fn iceberg::arrow::ArrowReaderOptions::serialize<__S>(&self, __serializer: __S) -> core::result::Result<<__S as serde_core::ser::Serializer>::Ok, <__S as serde_core::ser::Serializer>::Error> where __S: serde_core::ser::Serializer +impl<'de> serde_core::de::Deserialize<'de> for iceberg::arrow::ArrowReaderOptions where iceberg::arrow::ArrowReaderOptions: core::default::Default +pub fn iceberg::arrow::ArrowReaderOptions::deserialize<__D>(__deserializer: __D) -> core::result::Result::Error> where __D: serde_core::de::Deserializer<'de> pub struct iceberg::arrow::PartitionValueCalculator impl iceberg::arrow::PartitionValueCalculator pub fn iceberg::arrow::PartitionValueCalculator::calculate(&self, batch: &arrow_array::record_batch::RecordBatch) -> iceberg::Result @@ -104,27 +122,6 @@ pub struct iceberg::arrow::ScanResult impl iceberg::scan::ScanResult pub fn iceberg::scan::ScanResult::metrics(&self) -> &iceberg::scan::ScanMetrics pub fn iceberg::scan::ScanResult::stream(self) -> iceberg::scan::ArrowRecordBatchStream -pub struct iceberg::arrow::TableScanReaderOptions -impl iceberg::arrow::TableScanReaderOptions -pub fn iceberg::arrow::TableScanReaderOptions::apply_to(&self, builder: iceberg::arrow::ArrowReaderBuilder) -> iceberg::arrow::ArrowReaderBuilder -pub fn iceberg::arrow::TableScanReaderOptions::batch_size(&self) -> core::option::Option -pub fn iceberg::arrow::TableScanReaderOptions::bloom_filter_enabled(&self) -> bool -pub fn iceberg::arrow::TableScanReaderOptions::data_file_concurrency_limit(&self) -> usize -pub fn iceberg::arrow::TableScanReaderOptions::row_group_filtering_enabled(&self) -> bool -pub fn iceberg::arrow::TableScanReaderOptions::row_selection_enabled(&self) -> bool -impl core::clone::Clone for iceberg::arrow::TableScanReaderOptions -pub fn iceberg::arrow::TableScanReaderOptions::clone(&self) -> iceberg::arrow::TableScanReaderOptions -impl core::cmp::Eq for iceberg::arrow::TableScanReaderOptions -impl core::cmp::PartialEq for iceberg::arrow::TableScanReaderOptions -pub fn iceberg::arrow::TableScanReaderOptions::eq(&self, other: &iceberg::arrow::TableScanReaderOptions) -> bool -impl core::fmt::Debug for iceberg::arrow::TableScanReaderOptions -pub fn iceberg::arrow::TableScanReaderOptions::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result -impl core::marker::Copy for iceberg::arrow::TableScanReaderOptions -impl core::marker::StructuralPartialEq for iceberg::arrow::TableScanReaderOptions -impl serde_core::ser::Serialize for iceberg::arrow::TableScanReaderOptions -pub fn iceberg::arrow::TableScanReaderOptions::serialize<__S>(&self, __serializer: __S) -> core::result::Result<<__S as serde_core::ser::Serializer>::Ok, <__S as serde_core::ser::Serializer>::Error> where __S: serde_core::ser::Serializer -impl<'de> serde_core::de::Deserialize<'de> for iceberg::arrow::TableScanReaderOptions -pub fn iceberg::arrow::TableScanReaderOptions::deserialize<__D>(__deserializer: __D) -> core::result::Result::Error> where __D: serde_core::de::Deserializer<'de> pub const iceberg::arrow::DEFAULT_MAP_FIELD_NAME: &str pub const iceberg::arrow::PROJECTED_PARTITION_VALUE_COLUMN: &str pub const iceberg::arrow::UTC_TIME_ZONE: &str @@ -1457,9 +1454,8 @@ pub struct iceberg::scan::TableScan impl iceberg::scan::TableScan pub fn iceberg::scan::TableScan::column_names(&self) -> core::option::Option<&[alloc::string::String]> pub async fn iceberg::scan::TableScan::plan_files(&self) -> iceberg::Result -pub fn iceberg::scan::TableScan::reader_options(&self) -> iceberg::arrow::TableScanReaderOptions pub fn iceberg::scan::TableScan::snapshot(&self) -> core::option::Option<&iceberg::spec::SnapshotRef> -pub async fn iceberg::scan::TableScan::to_arrow(&self) -> iceberg::Result +pub async fn iceberg::scan::TableScan::to_arrow(&self, options: iceberg::arrow::ArrowReaderOptions) -> iceberg::Result impl core::fmt::Debug for iceberg::scan::TableScan pub fn iceberg::scan::TableScan::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result pub struct iceberg::scan::TableScanBuilder<'a> @@ -1470,15 +1466,10 @@ pub fn iceberg::scan::TableScanBuilder<'a>::select(self, column_names: impl core pub fn iceberg::scan::TableScanBuilder<'a>::select_all(self) -> Self pub fn iceberg::scan::TableScanBuilder<'a>::select_empty(self) -> Self pub fn iceberg::scan::TableScanBuilder<'a>::snapshot_id(self, snapshot_id: i64) -> Self -pub fn iceberg::scan::TableScanBuilder<'a>::with_batch_size(self, batch_size: core::option::Option) -> Self -pub fn iceberg::scan::TableScanBuilder<'a>::with_bloom_filter_enabled(self, bloom_filter_enabled: bool) -> Self pub fn iceberg::scan::TableScanBuilder<'a>::with_case_sensitive(self, case_sensitive: bool) -> Self pub fn iceberg::scan::TableScanBuilder<'a>::with_concurrency_limit(self, limit: usize) -> Self -pub fn iceberg::scan::TableScanBuilder<'a>::with_data_file_concurrency_limit(self, limit: usize) -> Self pub fn iceberg::scan::TableScanBuilder<'a>::with_filter(self, predicate: iceberg::expr::Predicate) -> Self pub fn iceberg::scan::TableScanBuilder<'a>::with_manifest_entry_concurrency_limit(self, limit: usize) -> Self -pub fn iceberg::scan::TableScanBuilder<'a>::with_row_group_filtering_enabled(self, row_group_filtering_enabled: bool) -> Self -pub fn iceberg::scan::TableScanBuilder<'a>::with_row_selection_enabled(self, row_selection_enabled: bool) -> Self pub type iceberg::scan::ArrowRecordBatchStream = futures_core::stream::BoxStream<'static, iceberg::Result> pub type iceberg::scan::FileScanTaskStream = futures_core::stream::BoxStream<'static, iceberg::Result> pub mod iceberg::sensitive diff --git a/crates/iceberg/src/arrow/reader/mod.rs b/crates/iceberg/src/arrow/reader/mod.rs index e779ae025a..a6aebf37fc 100644 --- a/crates/iceberg/src/arrow/reader/mod.rs +++ b/crates/iceberg/src/arrow/reader/mod.rs @@ -36,27 +36,27 @@ const DEFAULT_METADATA_SIZE_HINT: usize = 512 * 1024; mod file_reader; mod options; +mod parquet_options; mod pipeline; mod positional_deletes; mod predicate_visitor; mod projection; mod row_filter; mod row_lineage; -mod scan_options; pub use file_reader::ArrowFileReader; -pub(crate) use options::ParquetReadOptions; +pub use options::{ArrowReaderOptions, ArrowReaderOptionsBuilder}; +pub(crate) use parquet_options::ParquetReadOptions; use predicate_visitor::{CollectFieldIdVisitor, PredicateConverter}; use projection::{ add_fallback_field_ids_to_arrow_schema, apply_name_mapping_to_arrow_schema, find_leaf_by_field_id, }; -pub use scan_options::TableScanReaderOptions; /// Builder to create ArrowReader pub struct ArrowReaderBuilder { batch_size: Option, file_io: FileIO, - concurrency_limit_data_files: usize, + concurrency_limit_data_files: Option, row_group_filtering_enabled: bool, row_selection_enabled: bool, bloom_filter_enabled: bool, @@ -67,12 +67,10 @@ pub struct ArrowReaderBuilder { impl ArrowReaderBuilder { /// Create a new ArrowReaderBuilder pub fn new(file_io: FileIO, runtime: Runtime) -> Self { - let num_cpus = available_parallelism().get(); - ArrowReaderBuilder { batch_size: None, file_io, - concurrency_limit_data_files: num_cpus, + concurrency_limit_data_files: None, row_group_filtering_enabled: true, row_selection_enabled: false, bloom_filter_enabled: false, @@ -83,7 +81,7 @@ impl ArrowReaderBuilder { /// Sets the max number of in flight data files that are being fetched pub fn with_data_file_concurrency_limit(mut self, val: usize) -> Self { - self.concurrency_limit_data_files = val; + self.concurrency_limit_data_files = Some(val); self } @@ -149,15 +147,18 @@ impl ArrowReaderBuilder { /// Build the ArrowReader. pub fn build(self) -> ArrowReader { + let concurrency_limit_data_files = self + .concurrency_limit_data_files + .unwrap_or_else(|| available_parallelism().get()); ArrowReader { batch_size: self.batch_size, file_io: self.file_io.clone(), delete_file_loader: CachingDeleteFileLoader::new( self.file_io.clone(), - self.concurrency_limit_data_files, + concurrency_limit_data_files, self.runtime.clone(), ), - concurrency_limit_data_files: self.concurrency_limit_data_files, + concurrency_limit_data_files, row_group_filtering_enabled: self.row_group_filtering_enabled, row_selection_enabled: self.row_selection_enabled, bloom_filter_enabled: self.bloom_filter_enabled, diff --git a/crates/iceberg/src/arrow/reader/options.rs b/crates/iceberg/src/arrow/reader/options.rs index ae6a3ed18e..df094ef3b9 100644 --- a/crates/iceberg/src/arrow/reader/options.rs +++ b/crates/iceberg/src/arrow/reader/options.rs @@ -15,70 +15,118 @@ // specific language governing permissions and limitations // under the License. -//! Tunables for Parquet file I/O used by `ArrowReader`. - +use serde::{Deserialize, Serialize}; use typed_builder::TypedBuilder; -use super::{ - DEFAULT_METADATA_SIZE_HINT, DEFAULT_RANGE_COALESCE_BYTES, DEFAULT_RANGE_FETCH_CONCURRENCY, -}; +use super::ArrowReaderBuilder; -/// Options for tuning Parquet file I/O. -#[derive(Clone, Copy, Debug, TypedBuilder)] -#[builder(field_defaults(setter(prefix = "with_")))] -pub(crate) struct ParquetReadOptions { - /// Number of bytes to prefetch for parsing the Parquet metadata. - /// - /// This hint can help reduce the number of fetch requests. For more details see the - /// [ParquetMetaDataReader documentation](https://docs.rs/parquet/latest/parquet/file/metadata/struct.ParquetMetaDataReader.html#method.with_prefetch_hint). - /// - /// Defaults to 512 KiB, matching DataFusion's default `ParquetOptions::metadata_size_hint`. - #[builder(default = Some(DEFAULT_METADATA_SIZE_HINT))] - pub(crate) metadata_size_hint: Option, - /// Gap threshold for merging nearby byte ranges into a single request. - /// Ranges with gaps smaller than this value will be coalesced. - /// - /// Defaults to 1 MiB, matching object_store's `OBJECT_STORE_COALESCE_DEFAULT`. - #[builder(default = DEFAULT_RANGE_COALESCE_BYTES)] - pub(crate) range_coalesce_bytes: u64, - /// Maximum number of merged byte ranges to fetch concurrently. - /// - /// Defaults to 10, matching object_store's `OBJECT_STORE_COALESCE_PARALLEL`. - #[builder(default = DEFAULT_RANGE_FETCH_CONCURRENCY)] - pub(crate) range_fetch_concurrency: usize, - /// Whether to preload the column index when reading Parquet metadata. - #[builder(default = true)] - pub(crate) preload_column_index: bool, - /// Whether to preload the offset index when reading Parquet metadata. +/// Configures Arrow reads independently of file planning and local I/O resources. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, TypedBuilder)] +#[serde(default)] +#[builder(field_defaults(default, setter(prefix = "with_")))] +pub struct ArrowReaderOptions { + /// Desired batch size, or the reader default when unset. + batch_size: Option, + /// Maximum in-flight data files, or execution-host parallelism when unset. + data_file_concurrency_limit: Option, + /// Whether to prune row groups using their statistics. #[builder(default = true)] - pub(crate) preload_offset_index: bool, - /// Whether to preload the page index when reading Parquet metadata. - #[builder(default = false)] - pub(crate) preload_page_index: bool, + row_group_filtering_enabled: bool, + /// Whether to use page indexes for row selection. + row_selection_enabled: bool, + /// Whether to use bloom filters for row-group pruning. + bloom_filter_enabled: bool, } -impl ParquetReadOptions { - pub(crate) fn metadata_size_hint(&self) -> Option { - self.metadata_size_hint - } - - pub(crate) fn range_coalesce_bytes(&self) -> u64 { - self.range_coalesce_bytes +impl Default for ArrowReaderOptions { + fn default() -> Self { + Self::builder().build() } +} - pub(crate) fn range_fetch_concurrency(&self) -> usize { - self.range_fetch_concurrency +impl ArrowReaderBuilder { + /// Replaces reader settings while preserving local resources and Parquet I/O tuning. + /// Unset batch size and concurrency restore the defaults used at reader construction. + pub fn with_options(mut self, options: ArrowReaderOptions) -> Self { + self.batch_size = options.batch_size; + self.concurrency_limit_data_files = options.data_file_concurrency_limit; + self.row_group_filtering_enabled = options.row_group_filtering_enabled; + self.row_selection_enabled = options.row_selection_enabled; + self.bloom_filter_enabled = options.bloom_filter_enabled; + self } +} - pub(crate) fn preload_column_index(&self) -> bool { - self.preload_column_index - } +#[cfg(test)] +mod tests { + use super::*; + use crate::io::FileIO; + use crate::test_utils::test_runtime; + use crate::util::available_parallelism; - pub(crate) fn preload_offset_index(&self) -> bool { - self.preload_offset_index + #[test] + fn missing_options_use_defaults_and_unknown_options_are_ignored() { + let decoded: ArrowReaderOptions = + serde_json::from_str(r#"{"future_option":true}"#).unwrap(); + assert_eq!(decoded, ArrowReaderOptions::default()); + let decoded: ArrowReaderOptions = serde_json::from_str(r#"{"batch_size":17}"#).unwrap(); + assert_eq!( + decoded, + ArrowReaderOptions::builder() + .with_batch_size(Some(17)) + .build() + ); + assert_eq!( + serde_json::to_value(ArrowReaderOptions::default()).unwrap()["data_file_concurrency_limit"], + serde_json::Value::Null + ); } - pub(crate) fn preload_page_index(&self) -> bool { - self.preload_page_index + #[test] + fn options_restore_local_defaults_or_apply_explicit_settings() { + for explicit in [false, true] { + let options = if explicit { + ArrowReaderOptions::builder() + .with_batch_size(Some(17)) + .with_data_file_concurrency_limit(Some(3)) + .with_row_group_filtering_enabled(false) + .with_row_selection_enabled(true) + .with_bloom_filter_enabled(true) + .build() + } else { + ArrowReaderOptions::default() + }; + let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) + .with_batch_size(1024) + .with_data_file_concurrency_limit(99) + .with_metadata_size_hint(4096) + .with_range_coalesce_bytes(8192) + .with_range_fetch_concurrency(2) + .with_options(options) + .build(); + let expected = if explicit { + (Some(17), 3, false, true, true) + } else { + (None, available_parallelism().get(), true, false, false) + }; + assert_eq!( + ( + reader.batch_size, + reader.concurrency_limit_data_files, + reader.row_group_filtering_enabled, + reader.row_selection_enabled, + reader.bloom_filter_enabled + ), + expected + ); + assert_eq!( + ( + reader.parquet_read_options.metadata_size_hint(), + reader.parquet_read_options.range_coalesce_bytes(), + reader.parquet_read_options.range_fetch_concurrency() + ), + (Some(4096), 8192, 2) + ); + } } } diff --git a/crates/iceberg/src/arrow/reader/parquet_options.rs b/crates/iceberg/src/arrow/reader/parquet_options.rs new file mode 100644 index 0000000000..ae6a3ed18e --- /dev/null +++ b/crates/iceberg/src/arrow/reader/parquet_options.rs @@ -0,0 +1,84 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Tunables for Parquet file I/O used by `ArrowReader`. + +use typed_builder::TypedBuilder; + +use super::{ + DEFAULT_METADATA_SIZE_HINT, DEFAULT_RANGE_COALESCE_BYTES, DEFAULT_RANGE_FETCH_CONCURRENCY, +}; + +/// Options for tuning Parquet file I/O. +#[derive(Clone, Copy, Debug, TypedBuilder)] +#[builder(field_defaults(setter(prefix = "with_")))] +pub(crate) struct ParquetReadOptions { + /// Number of bytes to prefetch for parsing the Parquet metadata. + /// + /// This hint can help reduce the number of fetch requests. For more details see the + /// [ParquetMetaDataReader documentation](https://docs.rs/parquet/latest/parquet/file/metadata/struct.ParquetMetaDataReader.html#method.with_prefetch_hint). + /// + /// Defaults to 512 KiB, matching DataFusion's default `ParquetOptions::metadata_size_hint`. + #[builder(default = Some(DEFAULT_METADATA_SIZE_HINT))] + pub(crate) metadata_size_hint: Option, + /// Gap threshold for merging nearby byte ranges into a single request. + /// Ranges with gaps smaller than this value will be coalesced. + /// + /// Defaults to 1 MiB, matching object_store's `OBJECT_STORE_COALESCE_DEFAULT`. + #[builder(default = DEFAULT_RANGE_COALESCE_BYTES)] + pub(crate) range_coalesce_bytes: u64, + /// Maximum number of merged byte ranges to fetch concurrently. + /// + /// Defaults to 10, matching object_store's `OBJECT_STORE_COALESCE_PARALLEL`. + #[builder(default = DEFAULT_RANGE_FETCH_CONCURRENCY)] + pub(crate) range_fetch_concurrency: usize, + /// Whether to preload the column index when reading Parquet metadata. + #[builder(default = true)] + pub(crate) preload_column_index: bool, + /// Whether to preload the offset index when reading Parquet metadata. + #[builder(default = true)] + pub(crate) preload_offset_index: bool, + /// Whether to preload the page index when reading Parquet metadata. + #[builder(default = false)] + pub(crate) preload_page_index: bool, +} + +impl ParquetReadOptions { + pub(crate) fn metadata_size_hint(&self) -> Option { + self.metadata_size_hint + } + + pub(crate) fn range_coalesce_bytes(&self) -> u64 { + self.range_coalesce_bytes + } + + pub(crate) fn range_fetch_concurrency(&self) -> usize { + self.range_fetch_concurrency + } + + pub(crate) fn preload_column_index(&self) -> bool { + self.preload_column_index + } + + pub(crate) fn preload_offset_index(&self) -> bool { + self.preload_offset_index + } + + pub(crate) fn preload_page_index(&self) -> bool { + self.preload_page_index + } +} diff --git a/crates/iceberg/src/arrow/reader/scan_options.rs b/crates/iceberg/src/arrow/reader/scan_options.rs deleted file mode 100644 index a6ea5c7fac..0000000000 --- a/crates/iceberg/src/arrow/reader/scan_options.rs +++ /dev/null @@ -1,116 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -use serde::{Deserialize, Serialize}; - -use super::ArrowReaderBuilder; - -/// Reader settings captured from a [`TableScan`](crate::scan::TableScan). -/// -/// Contains no I/O handles, runtime, or planning state. Apply these options to a -/// locally constructed builder, then build one reader and clone it across -/// partitions to share its delete-file cache. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -pub struct TableScanReaderOptions { - pub(crate) batch_size: Option, - pub(crate) data_file_concurrency_limit: usize, - pub(crate) row_group_filtering_enabled: bool, - pub(crate) row_selection_enabled: bool, - pub(crate) bloom_filter_enabled: bool, -} - -impl TableScanReaderOptions { - /// The requested record batch size, or the reader default. - pub fn batch_size(&self) -> Option { - self.batch_size - } - - /// The maximum number of data files read concurrently. - pub fn data_file_concurrency_limit(&self) -> usize { - self.data_file_concurrency_limit - } - - /// Whether row-group filtering is enabled. - pub fn row_group_filtering_enabled(&self) -> bool { - self.row_group_filtering_enabled - } - - /// Whether page-index row selection is enabled. - pub fn row_selection_enabled(&self) -> bool { - self.row_selection_enabled - } - - /// Whether bloom-filter row-group filtering is enabled. - pub fn bloom_filter_enabled(&self) -> bool { - self.bloom_filter_enabled - } - - /// Applies all scan reader settings, preserving the builder's local resources - /// and Parquet I/O tuning. An unset batch size restores the reader default. - pub fn apply_to(&self, mut builder: ArrowReaderBuilder) -> ArrowReaderBuilder { - builder.batch_size = self.batch_size; - builder.concurrency_limit_data_files = self.data_file_concurrency_limit; - builder.row_group_filtering_enabled = self.row_group_filtering_enabled; - builder.row_selection_enabled = self.row_selection_enabled; - builder.bloom_filter_enabled = self.bloom_filter_enabled; - builder - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::io::FileIO; - use crate::test_utils::test_runtime; - - #[test] - fn applies_scan_settings_and_preserves_local_io_tuning() { - for batch_size in [None, Some(17)] { - let options = TableScanReaderOptions { - batch_size, - data_file_concurrency_limit: 3, - row_group_filtering_enabled: false, - row_selection_enabled: true, - bloom_filter_enabled: true, - }; - let builder = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) - .with_batch_size(1024) - .with_metadata_size_hint(4096) - .with_range_coalesce_bytes(8192) - .with_range_fetch_concurrency(2); - let reader = options.apply_to(builder).build(); - assert_eq!( - ( - reader.batch_size, - reader.concurrency_limit_data_files, - reader.row_group_filtering_enabled, - reader.row_selection_enabled, - reader.bloom_filter_enabled - ), - (batch_size, 3, false, true, true), - ); - assert_eq!( - ( - reader.parquet_read_options.metadata_size_hint(), - reader.parquet_read_options.range_coalesce_bytes(), - reader.parquet_read_options.range_fetch_concurrency() - ), - (Some(4096), 8192, 2), - ); - } - } -} diff --git a/crates/iceberg/src/lib.rs b/crates/iceberg/src/lib.rs index 72c2acd3e6..17fbfb0e38 100644 --- a/crates/iceberg/src/lib.rs +++ b/crates/iceberg/src/lib.rs @@ -51,7 +51,7 @@ //! .scan() //! .select(["name", "id"]) //! .build()? -//! .to_arrow() +//! .to_arrow(Default::default()) //! .await?; //! //! // Consume this stream like arrow record batch stream. diff --git a/crates/iceberg/src/scan/mod.rs b/crates/iceberg/src/scan/mod.rs index e7fd208c99..803f3f4ab7 100644 --- a/crates/iceberg/src/scan/mod.rs +++ b/crates/iceberg/src/scan/mod.rs @@ -33,7 +33,7 @@ use futures::stream::BoxStream; use futures::{SinkExt, StreamExt, TryStreamExt}; pub use task::*; -use crate::arrow::{ArrowReaderBuilder, TableScanReaderOptions}; +use crate::arrow::{ArrowReaderBuilder, ArrowReaderOptions}; pub use crate::arrow::{ScanMetrics, ScanResult}; use crate::delete_file_index::DeleteFileIndex; use crate::error::invalid_data; @@ -143,15 +143,10 @@ pub struct TableScanBuilder<'a> { // Defaults to none which means select all columns column_names: Option>, snapshot_selection: Option, - batch_size: Option, case_sensitive: bool, filter: Option, - concurrency_limit_data_files: usize, concurrency_limit_manifest_entries: usize, concurrency_limit_manifest_files: usize, - row_group_filtering_enabled: bool, - row_selection_enabled: bool, - bloom_filter_enabled: bool, } impl<'a> TableScanBuilder<'a> { @@ -162,25 +157,13 @@ impl<'a> TableScanBuilder<'a> { table, column_names: None, snapshot_selection: None, - batch_size: None, case_sensitive: true, filter: None, - concurrency_limit_data_files: num_cpus, concurrency_limit_manifest_entries: num_cpus, concurrency_limit_manifest_files: num_cpus, - row_group_filtering_enabled: true, - row_selection_enabled: false, - bloom_filter_enabled: false, } } - /// Sets the desired size of batches in the response - /// to something other than the default - pub fn with_batch_size(mut self, batch_size: Option) -> Self { - self.batch_size = batch_size; - self - } - /// Sets the scan's case sensitivity pub fn with_case_sensitive(mut self, case_sensitive: bool) -> Self { self.case_sensitive = case_sensitive; @@ -259,13 +242,6 @@ impl<'a> TableScanBuilder<'a> { pub fn with_concurrency_limit(mut self, limit: usize) -> Self { self.concurrency_limit_manifest_files = limit; self.concurrency_limit_manifest_entries = limit; - self.concurrency_limit_data_files = limit; - self - } - - /// Sets the data file concurrency limit for this scan - pub fn with_data_file_concurrency_limit(mut self, limit: usize) -> Self { - self.concurrency_limit_data_files = limit; self } @@ -275,52 +251,6 @@ impl<'a> TableScanBuilder<'a> { self } - /// Determines whether to enable row group filtering. - /// When enabled, if a read is performed with a filter predicate, - /// then the metadata for each row group in the parquet file is - /// evaluated against the filter predicate and row groups - /// that cant contain matching rows will be skipped entirely. - /// - /// Defaults to enabled, as it generally improves performance or - /// keeps it the same, with performance degradation unlikely. - pub fn with_row_group_filtering_enabled(mut self, row_group_filtering_enabled: bool) -> Self { - self.row_group_filtering_enabled = row_group_filtering_enabled; - self - } - - /// Determines whether to enable row selection. - /// When enabled, if a read is performed with a filter predicate, - /// then (for row groups that have not been skipped) the page index - /// for each row group in a parquet file is parsed and evaluated - /// against the filter predicate to determine if ranges of rows - /// within a row group can be skipped, based upon the page-level - /// statistics for each column. - /// - /// Defaults to being disabled. Enabling requires parsing the parquet page - /// index, which can be slow enough that parsing the page index outweighs any - /// gains from the reduced number of rows that need scanning. - /// It is recommended to experiment with partitioning, sorting, row group size, - /// page size, and page row limit Iceberg settings on the table being scanned in - /// order to get the best performance from using row selection. - pub fn with_row_selection_enabled(mut self, row_selection_enabled: bool) -> Self { - self.row_selection_enabled = row_selection_enabled; - self - } - - /// Determines whether to enable bloom filter-based row group filtering. - /// - /// When enabled, if a read is performed with an equality or IN predicate, - /// the bloom filter for relevant columns in each row group is read and - /// checked. Row groups where the bloom filter proves the value is absent - /// are skipped entirely. - /// - /// Defaults to disabled, as reading bloom filters requires additional I/O - /// per column per row group. - pub fn with_bloom_filter_enabled(mut self, bloom_filter_enabled: bool) -> Self { - self.bloom_filter_enabled = bloom_filter_enabled; - self - } - /// Build the table scan. pub fn build(self) -> Result { let selected_snapshot = match self.snapshot_selection { @@ -350,16 +280,11 @@ impl<'a> TableScanBuilder<'a> { None => { let Some(current_snapshot_id) = self.table.metadata().current_snapshot() else { return Ok(TableScan { - batch_size: self.batch_size, column_names: self.column_names, file_io: self.table.file_io().clone(), plan_context: None, - concurrency_limit_data_files: self.concurrency_limit_data_files, concurrency_limit_manifest_entries: self.concurrency_limit_manifest_entries, concurrency_limit_manifest_files: self.concurrency_limit_manifest_files, - row_group_filtering_enabled: self.row_group_filtering_enabled, - row_selection_enabled: self.row_selection_enabled, - bloom_filter_enabled: self.bloom_filter_enabled, runtime: self.table.runtime().clone(), }); }; @@ -414,16 +339,11 @@ impl<'a> TableScanBuilder<'a> { }; Ok(TableScan { - batch_size: self.batch_size, column_names: self.column_names, file_io: self.table.file_io().clone(), plan_context: Some(plan_context), - concurrency_limit_data_files: self.concurrency_limit_data_files, concurrency_limit_manifest_entries: self.concurrency_limit_manifest_entries, concurrency_limit_manifest_files: self.concurrency_limit_manifest_files, - row_group_filtering_enabled: self.row_group_filtering_enabled, - row_selection_enabled: self.row_selection_enabled, - bloom_filter_enabled: self.bloom_filter_enabled, runtime: self.table.runtime().clone(), }) } @@ -436,7 +356,6 @@ pub struct TableScan { /// /// If this is None, then the scan contains no rows. plan_context: Option, - batch_size: Option, file_io: FileIO, column_names: Option>, /// The maximum number of manifest files that will be @@ -447,14 +366,6 @@ pub struct TableScan { /// be processed in parallel concurrency_limit_manifest_entries: usize, - /// The maximum number of [`ManifestEntry`]s that will - /// be processed in parallel - concurrency_limit_data_files: usize, - - row_group_filtering_enabled: bool, - row_selection_enabled: bool, - bloom_filter_enabled: bool, - runtime: Runtime, } @@ -601,52 +512,14 @@ impl TableScan { } /// Returns an [`ArrowRecordBatchStream`]. - pub async fn to_arrow(&self) -> Result { - self.reader_options() - .apply_to(ArrowReaderBuilder::new( - self.file_io.clone(), - self.runtime.clone(), - )) + pub async fn to_arrow(&self, options: ArrowReaderOptions) -> Result { + ArrowReaderBuilder::new(self.file_io.clone(), self.runtime.clone()) + .with_options(options) .build() .read(self.plan_files().await?) .map(|result| result.stream()) } - /// Captures reader settings independently of this scan's planning state. - /// - /// Projection, predicates, and delete references are carried by the planned - /// file tasks; manifest-planning concurrency is not a reader setting. - /// - /// ```no_run - /// # use futures::{TryStreamExt, stream}; - /// # use iceberg::{Result, Runtime, arrow::ArrowReaderBuilder, io::FileIO, table::Table}; - /// # async fn read(table: &Table, local_io: FileIO, local_runtime: Runtime) -> Result<()> { - /// let scan = table.scan().with_batch_size(Some(1024)).build()?; - /// let options = scan.reader_options(); - /// let tasks: Vec<_> = scan.plan_files().await?.try_collect().await?; - /// drop(scan); - /// - /// let reader = options - /// .apply_to(ArrowReaderBuilder::new(local_io, local_runtime)) - /// .build(); - /// let batches: Vec<_> = reader - /// .read(Box::pin(stream::iter(tasks.into_iter().map(Ok))))? - /// .stream() - /// .try_collect() - /// .await?; - /// # Ok(()) - /// # } - /// ``` - pub fn reader_options(&self) -> TableScanReaderOptions { - TableScanReaderOptions { - batch_size: self.batch_size, - data_file_concurrency_limit: self.concurrency_limit_data_files, - row_group_filtering_enabled: self.row_group_filtering_enabled, - row_selection_enabled: self.row_selection_enabled, - bloom_filter_enabled: self.bloom_filter_enabled, - } - } - /// Returns a reference to the column names of the table scan. pub fn column_names(&self) -> Option<&[String]> { self.column_names.as_deref() @@ -778,7 +651,7 @@ mod tests { use uuid::Uuid; use super::resolve_field_id; - use crate::arrow::{ArrowReaderBuilder, TableScanReaderOptions}; + use crate::arrow::{ArrowReaderBuilder, ArrowReaderOptions}; use crate::expr::{BoundPredicate, Reference}; use crate::io::FileIO; use crate::metadata_columns::{ @@ -920,7 +793,7 @@ mod tests { .unwrap(); let batches: Vec<_> = table_scan - .to_arrow() + .to_arrow(ArrowReaderOptions::default()) .await .unwrap() .try_collect() @@ -976,7 +849,6 @@ mod tests { let table_scan = table .scan() .snapshot_id(3051729675574597004) - .with_row_selection_enabled(true) .build() .unwrap(); assert_eq!( @@ -1317,7 +1189,13 @@ mod tests { .unwrap(); tasks.sort_by_key(|task| task.data_file_path().to_string()); planned_files.push(tasks); - let batches: Vec<_> = scan.to_arrow().await.unwrap().try_collect().await.unwrap(); + let batches: Vec<_> = scan + .to_arrow(ArrowReaderOptions::default()) + .await + .unwrap() + .try_collect() + .await + .unwrap(); assert!(!batches.is_empty()); schemas.push(batches[0].schema()); let mut values: Vec = batches @@ -1591,7 +1469,13 @@ mod tests { #[tokio::test] async fn test_plan_files_on_table_without_any_snapshots() { let table = TableTestFixture::new_empty().table; - let batch_stream = table.scan().build().unwrap().to_arrow().await.unwrap(); + let batch_stream = table + .scan() + .build() + .unwrap() + .to_arrow(ArrowReaderOptions::default()) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); assert!(batches.is_empty()); } @@ -1602,12 +1486,7 @@ mod tests { fixture.setup_manifest_files().await; // Create table scan for current snapshot and plan files - let table_scan = fixture - .table - .scan() - .with_row_selection_enabled(true) - .build() - .unwrap(); + let table_scan = fixture.table.scan().build().unwrap(); let mut tasks = table_scan .plan_files() @@ -1964,7 +1843,7 @@ mod tests { .build() .unwrap(); let batches: Vec<_> = table_scan - .to_arrow() + .to_arrow(ArrowReaderOptions::default()) .await .unwrap() .try_collect() @@ -1989,14 +1868,16 @@ mod tests { fixture.setup_manifest_files().await; // Create table scan for current snapshot and plan files - let table_scan = fixture - .table - .scan() - .with_row_selection_enabled(true) - .build() - .unwrap(); + let table_scan = fixture.table.scan().build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); @@ -2010,58 +1891,57 @@ mod tests { async fn reader_options_preserve_reads_after_dropping_scan() { let mut fixture = TableTestFixture::new(); fixture.setup_multi_row_group_manifest(&[150, 299]).await; - for enabled in [false, true] { - let scan = fixture - .table - .scan() - .select(["y"]) - .with_filter(Reference::new("y").greater_than(Datum::long(1100))) - .with_batch_size(Some(17)) - .with_data_file_concurrency_limit(1) - .with_row_group_filtering_enabled(enabled) - .with_row_selection_enabled(enabled) - .with_bloom_filter_enabled(enabled) - .build() - .unwrap(); - let expected: Vec<_> = scan.to_arrow().await.unwrap().try_collect().await.unwrap(); - let tasks: Vec<_> = scan - .plan_files() - .await - .unwrap() - .try_collect() - .await - .unwrap(); - let options = scan.reader_options(); - let serialized = serde_json::to_value(options).unwrap(); - assert_eq!( - serialized, - serde_json::json!({ - "batch_size": 17, - "data_file_concurrency_limit": 1, - "row_group_filtering_enabled": enabled, - "row_selection_enabled": enabled, - "bloom_filter_enabled": enabled, - }) - ); - let options: TableScanReaderOptions = serde_json::from_value(serialized).unwrap(); - drop(scan); - let reader = options - .apply_to(ArrowReaderBuilder::new( - FileIO::new_with_fs(), - Runtime::try_current().unwrap(), - )) + let scan = fixture + .table + .scan() + .select(["y"]) + .with_filter(Reference::new("y").greater_than(Datum::long(1100))) + .build() + .unwrap(); + let options = ArrowReaderOptions::builder() + .with_batch_size(Some(17)) + .with_data_file_concurrency_limit(Some(1)) + .with_row_selection_enabled(true) + .build(); + let expected: Vec<_> = scan + .to_arrow(options.clone()) + .await + .unwrap() + .try_collect() + .await + .unwrap(); + let tasks: Vec<_> = scan + .plan_files() + .await + .unwrap() + .try_collect() + .await + .unwrap(); + let serialized = serde_json::to_value(&options).unwrap(); + assert_eq!( + serialized, + serde_json::json!({ + "batch_size": 17, "data_file_concurrency_limit": 1, + "row_group_filtering_enabled": true, "row_selection_enabled": true, + "bloom_filter_enabled": false, + }) + ); + let options: ArrowReaderOptions = serde_json::from_value(serialized).unwrap(); + drop(scan); + let reader = + ArrowReaderBuilder::new(FileIO::new_with_fs(), Runtime::try_current().unwrap()) + .with_options(options) .build(); - let actual: Vec<_> = reader - .read(Box::pin(stream::iter(tasks.into_iter().map(Ok)))) - .unwrap() - .stream() - .try_collect() - .await - .unwrap(); - assert!(!actual.is_empty()); - assert!(actual.iter().all(|batch| batch.num_rows() <= 17)); - assert_eq!(actual, expected); - } + let actual: Vec<_> = reader + .read(Box::pin(stream::iter(tasks.into_iter().map(Ok)))) + .unwrap() + .stream() + .try_collect() + .await + .unwrap(); + assert!(!actual.is_empty()); + assert!(actual.iter().all(|batch| batch.num_rows() <= 17)); + assert_eq!(actual, expected); } #[tokio::test] @@ -2070,12 +1950,7 @@ mod tests { fixture.setup_manifest_files().await; // Create table scan for current snapshot and plan files - let table_scan = fixture - .table - .scan() - .with_row_selection_enabled(true) - .build() - .unwrap(); + let table_scan = fixture.table.scan().build().unwrap(); let mut plan_task: Vec<_> = table_scan .plan_files() @@ -2118,15 +1993,16 @@ mod tests { fixture.setup_manifest_files().await; // Create table scan for current snapshot and plan files - let table_scan = fixture - .table - .scan() - .select(["x", "z"]) - .with_row_selection_enabled(true) - .build() - .unwrap(); + let table_scan = fixture.table.scan().select(["x", "z"]).build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); @@ -2142,7 +2018,14 @@ mod tests { // test empty scan let table_scan = fixture.table.scan().select_empty().build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); assert_eq!(batches[0].num_columns(), 0); @@ -2157,12 +2040,17 @@ mod tests { // Filter: y < 3 let mut builder = fixture.table.scan(); let predicate = Reference::new("y").less_than(Datum::long(3)); - builder = builder - .with_filter(predicate) - .with_row_selection_enabled(true); + builder = builder.with_filter(predicate); let table_scan = builder.build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); @@ -2185,12 +2073,17 @@ mod tests { // Filter: y >= 5 let mut builder = fixture.table.scan(); let predicate = Reference::new("y").greater_than_or_equal_to(Datum::long(5)); - builder = builder - .with_filter(predicate) - .with_row_selection_enabled(true); + builder = builder.with_filter(predicate); let table_scan = builder.build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); @@ -2213,12 +2106,17 @@ mod tests { // Filter: dbl == 150.0 let mut builder = fixture.table.scan(); let predicate = Reference::new("dbl").equal_to(Datum::double(150.0f64)); - builder = builder - .with_filter(predicate) - .with_row_selection_enabled(true); + builder = builder.with_filter(predicate); let table_scan = builder.build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); @@ -2238,12 +2136,17 @@ mod tests { // Filter: i32 == 150 let mut builder = fixture.table.scan(); let predicate = Reference::new("i32").equal_to(Datum::int(150i32)); - builder = builder - .with_filter(predicate) - .with_row_selection_enabled(true); + builder = builder.with_filter(predicate); let table_scan = builder.build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); @@ -2263,12 +2166,17 @@ mod tests { // Filter: i64 == 150 let mut builder = fixture.table.scan(); let predicate = Reference::new("i64").equal_to(Datum::long(150i64)); - builder = builder - .with_filter(predicate) - .with_row_selection_enabled(true); + builder = builder.with_filter(predicate); let table_scan = builder.build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); @@ -2288,12 +2196,17 @@ mod tests { // Filter: bool == true let mut builder = fixture.table.scan(); let predicate = Reference::new("bool").equal_to(Datum::bool(true)); - builder = builder - .with_filter(predicate) - .with_row_selection_enabled(true); + builder = builder.with_filter(predicate); let table_scan = builder.build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); @@ -2313,12 +2226,17 @@ mod tests { // Filter: y is null let mut builder = fixture.table.scan(); let predicate = Reference::new("y").is_null(); - builder = builder - .with_filter(predicate) - .with_row_selection_enabled(true); + builder = builder.with_filter(predicate); let table_scan = builder.build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); assert_eq!(batches.len(), 0); @@ -2332,12 +2250,17 @@ mod tests { // Filter: y is not null let mut builder = fixture.table.scan(); let predicate = Reference::new("y").is_not_null(); - builder = builder - .with_filter(predicate) - .with_row_selection_enabled(true); + builder = builder.with_filter(predicate); let table_scan = builder.build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); assert_eq!(batches[0].num_rows(), 1024); @@ -2353,12 +2276,17 @@ mod tests { let predicate = Reference::new("y") .less_than(Datum::long(5)) .and(Reference::new("z").greater_than_or_equal_to(Datum::long(4))); - builder = builder - .with_filter(predicate) - .with_row_selection_enabled(true); + builder = builder.with_filter(predicate); let table_scan = builder.build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); assert_eq!(batches[0].num_rows(), 500); @@ -2389,12 +2317,17 @@ mod tests { let predicate = Reference::new("y") .less_than(Datum::long(5)) .or(Reference::new("z").greater_than_or_equal_to(Datum::long(4))); - builder = builder - .with_filter(predicate) - .with_row_selection_enabled(true); + builder = builder.with_filter(predicate); let table_scan = builder.build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); assert_eq!(batches[0].num_rows(), 1024); @@ -2426,12 +2359,17 @@ mod tests { // Filter: a STARTSWITH "Ice" let mut builder = fixture.table.scan(); let predicate = Reference::new("a").starts_with(Datum::string("Ice")); - builder = builder - .with_filter(predicate) - .with_row_selection_enabled(true); + builder = builder.with_filter(predicate); let table_scan = builder.build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); @@ -2450,12 +2388,17 @@ mod tests { // Filter: a NOT STARTSWITH "Ice" let mut builder = fixture.table.scan(); let predicate = Reference::new("a").not_starts_with(Datum::string("Ice")); - builder = builder - .with_filter(predicate) - .with_row_selection_enabled(true); + builder = builder.with_filter(predicate); let table_scan = builder.build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); @@ -2475,12 +2418,17 @@ mod tests { let mut builder = fixture.table.scan(); let predicate = Reference::new("a").is_in([Datum::string("Sioux"), Datum::string("Iceberg")]); - builder = builder - .with_filter(predicate) - .with_row_selection_enabled(true); + builder = builder.with_filter(predicate); let table_scan = builder.build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); @@ -2500,12 +2448,17 @@ mod tests { let mut builder = fixture.table.scan(); let predicate = Reference::new("a").is_not_in([Datum::string("Sioux"), Datum::string("Iceberg")]); - builder = builder - .with_filter(predicate) - .with_row_selection_enabled(true); + builder = builder.with_filter(predicate); let table_scan = builder.build().unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); @@ -2770,11 +2723,17 @@ mod tests { .table .scan() .select(["x", RESERVED_COL_NAME_FILE]) - .with_row_selection_enabled(true) .build() .unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); // Verify we have 2 columns: x and _file @@ -2829,11 +2788,17 @@ mod tests { .table .scan() .select(["x", RESERVED_COL_NAME_FILE, "z"]) - .with_row_selection_enabled(true) .build() .unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); assert_eq!(batches[0].num_columns(), 3); @@ -2860,11 +2825,17 @@ mod tests { .table .scan() .select([RESERVED_COL_NAME_FILE]) - .with_row_selection_enabled(true) .build() .unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); // Should have exactly 1 column @@ -2893,11 +2864,17 @@ mod tests { .table .scan() .select(["x", RESERVED_COL_NAME_FILE]) - .with_row_selection_enabled(true) .build() .unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); // Collect all unique file paths from the batches @@ -2938,11 +2915,17 @@ mod tests { .table .scan() .select([RESERVED_COL_NAME_FILE, "x", "y"]) - .with_row_selection_enabled(true) .build() .unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); assert_eq!(batches[0].num_columns(), 3); @@ -2964,11 +2947,17 @@ mod tests { .table .scan() .select(["x", "y", RESERVED_COL_NAME_FILE]) - .with_row_selection_enabled(true) .build() .unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); assert_eq!(batches[0].num_columns(), 3); @@ -2998,11 +2987,17 @@ mod tests { RESERVED_COL_NAME_FILE, // _file repeated "y", // y repeated ]) - .with_row_selection_enabled(true) .build() .unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); // Verify we have exactly 6 columns (duplicates are allowed and preserved) @@ -3232,11 +3227,17 @@ mod tests { .table .scan() .select(["x", RESERVED_COL_NAME_SPEC_ID, "z"]) - .with_row_selection_enabled(true) .build() .unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); // Verify we have 3 columns: x, _spec_id, and z @@ -3294,12 +3295,15 @@ mod tests { .table .scan() .select(["x", RESERVED_COL_NAME_LAST_UPDATED_SEQUENCE_NUMBER]) - .with_row_selection_enabled(true) .build() .unwrap(); let batches: Vec<_> = table_scan - .to_arrow() + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) .await .unwrap() .try_collect() @@ -3333,12 +3337,15 @@ mod tests { .table .scan() .select(["x", RESERVED_COL_NAME_LAST_UPDATED_SEQUENCE_NUMBER]) - .with_row_selection_enabled(true) .build() .unwrap(); let batches: Vec<_> = table_scan - .to_arrow() + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) .await .unwrap() .try_collect() @@ -3358,11 +3365,17 @@ mod tests { .table .scan() .select(["x", RESERVED_COL_NAME_SPEC_ID]) - .with_row_selection_enabled(true) .build() .unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); // Verify we have 2 columns: x and _spec_id @@ -3411,11 +3424,17 @@ mod tests { .table .scan() .select(["x", RESERVED_COL_NAME_SPEC_ID, "z"]) - .with_row_selection_enabled(true) .build() .unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); // Verify the x column exists and has correct data @@ -3466,11 +3485,17 @@ mod tests { .table .scan() .select(["x", RESERVED_COL_NAME_POS, RESERVED_COL_NAME_FILE]) - .with_row_selection_enabled(true) .build() .unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); assert_eq!(batches.len(), 2); @@ -3516,11 +3541,17 @@ mod tests { .scan() .select([RESERVED_COL_NAME_POS, "x", "y"]) .with_filter(predicate) - .with_row_selection_enabled(true) .build() .unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); assert_eq!(batches.len(), 2); @@ -3556,12 +3587,18 @@ mod tests { .table .scan() .select([RESERVED_COL_NAME_POS, "a", RESERVED_COL_NAME_POS, "x"]) - .with_row_selection_enabled(true) .with_filter(predicate) .build() .unwrap(); - let batch_stream = table_scan.to_arrow().await.unwrap(); + let batch_stream = table_scan + .to_arrow( + ArrowReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) + .await + .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); assert_eq!(batches.len(), 2); @@ -3629,7 +3666,7 @@ mod tests { .select(["x", RESERVED_COL_NAME_POS]) .build() .unwrap() - .to_arrow() + .to_arrow(ArrowReaderOptions::default()) .await .unwrap() .try_collect() @@ -3703,7 +3740,7 @@ mod tests { .select(["x", RESERVED_COL_NAME_POS]) .build() .unwrap() - .to_arrow() + .to_arrow(ArrowReaderOptions::default()) .await .unwrap() .try_collect() @@ -3755,10 +3792,9 @@ mod tests { .scan() .select(["y", RESERVED_COL_NAME_POS]) .with_filter(predicate) - .with_row_group_filtering_enabled(true) .build() .unwrap() - .to_arrow() + .to_arrow(ArrowReaderOptions::default()) .await .unwrap() .try_collect() diff --git a/crates/integration_tests/tests/conflict_commit_test.rs b/crates/integration_tests/tests/conflict_commit_test.rs index 57153d44f0..73da9736ef 100644 --- a/crates/integration_tests/tests/conflict_commit_test.rs +++ b/crates/integration_tests/tests/conflict_commit_test.rs @@ -119,7 +119,7 @@ async fn test_append_data_file_conflict() { .select_all() .build() .unwrap() - .to_arrow() + .to_arrow(Default::default()) .await .unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); diff --git a/crates/integration_tests/tests/read_evolved_schema.rs b/crates/integration_tests/tests/read_evolved_schema.rs index f7416be2d4..844ad81136 100644 --- a/crates/integration_tests/tests/read_evolved_schema.rs +++ b/crates/integration_tests/tests/read_evolved_schema.rs @@ -48,7 +48,7 @@ async fn test_evolved_schema() { let predicate = Reference::new("language").not_equal_to(Datum::string("Rust")); let scan = table.scan().with_filter(predicate).build(); - let batch_stream = scan.unwrap().to_arrow().await.unwrap(); + let batch_stream = scan.unwrap().to_arrow(Default::default()).await.unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); @@ -84,7 +84,7 @@ async fn test_evolved_schema() { // Reference::new("foo").not_equal_to(Datum::int(22)); let scan = table.scan().build(); - let batch_stream = scan.unwrap().to_arrow().await.unwrap(); + let batch_stream = scan.unwrap().to_arrow(Default::default()).await.unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); let mut actual = vec![ @@ -114,7 +114,7 @@ async fn test_evolved_schema() { .await .unwrap(); let scan = table.scan().build(); - let batch_stream = scan.unwrap().to_arrow().await.unwrap(); + let batch_stream = scan.unwrap().to_arrow(Default::default()).await.unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); let mut actual_foo = vec![ diff --git a/crates/integration_tests/tests/read_positional_deletes.rs b/crates/integration_tests/tests/read_positional_deletes.rs index fe8c9830d4..64d07f86fc 100644 --- a/crates/integration_tests/tests/read_positional_deletes.rs +++ b/crates/integration_tests/tests/read_positional_deletes.rs @@ -61,7 +61,7 @@ async fn test_read_table_with_positional_deletes() { assert_eq!(plan[0].deletes().len(), 1); // we should see two rows deleted, returning 10 rows instead of 12 - let batch_stream = scan.to_arrow().await.unwrap(); + let batch_stream = scan.to_arrow(Default::default()).await.unwrap(); let batches: Vec<_> = batch_stream.try_collect().await.unwrap(); let num_rows: usize = batches.iter().map(|v| v.num_rows()).sum(); assert_eq!(num_rows, 10); diff --git a/crates/storage/opendal/README.md b/crates/storage/opendal/README.md index ac3a8b62ef..87ebdbe171 100644 --- a/crates/storage/opendal/README.md +++ b/crates/storage/opendal/README.md @@ -80,7 +80,7 @@ async fn main() -> iceberg::Result<()> { .await?; let scan = table.scan().select_all().build()?; - let stream = scan.to_arrow().await?; + let stream = scan.to_arrow(Default::default()).await?; Ok(()) } From 5172999a5266ab6d0b8c5675eab9e30af6af19b4 Mon Sep 17 00:00:00 2001 From: "alex.bianchi" Date: Mon, 26 Oct 2026 10:59:49 -0400 Subject: [PATCH 03/15] feat(arrow): include Parquet tuning in shared reader options --- crates/iceberg/README.md | 10 +- crates/iceberg/public-api.txt | 23 +- crates/iceberg/src/arrow/reader/mod.rs | 122 +---------- crates/iceberg/src/arrow/reader/options.rs | 202 +++++++++++++++++- .../src/arrow/reader/parquet_options.rs | 21 +- crates/iceberg/src/scan/mod.rs | 9 +- 6 files changed, 248 insertions(+), 139 deletions(-) diff --git a/crates/iceberg/README.md b/crates/iceberg/README.md index 3e06db4719..f6688c0a52 100644 --- a/crates/iceberg/README.md +++ b/crates/iceberg/README.md @@ -73,6 +73,11 @@ Pass reader settings separately from file planning: let options = iceberg::arrow::ArrowReaderOptions::builder() .with_batch_size(Some(1024)) .with_row_selection_enabled(true) + .with_parquet_options( + iceberg::arrow::ParquetReadOptions::builder() + .with_metadata_size_hint(Some(512 * 1024)) + .build(), + ) .build(); let stream = table.scan().build()?.to_arrow(options).await?; ``` @@ -85,7 +90,10 @@ Use `to_arrow(Default::default())` for default settings. Scan limit data-file reads. An unset data-file limit uses execution-host parallelism. Locally constructed readers accept the same options through -`ArrowReaderBuilder::with_options(options)`. +`ArrowReaderBuilder::with_options(options)`, including Parquet I/O tuning. +This replaces the complete configuration; subsequent convenience setters override +individual settings. `with_parquet_options()` replaces only the nested Parquet +configuration. ## Storage Backends diff --git a/crates/iceberg/public-api.txt b/crates/iceberg/public-api.txt index 18c63e6ba2..35345f3eae 100644 --- a/crates/iceberg/public-api.txt +++ b/crates/iceberg/public-api.txt @@ -75,12 +75,12 @@ pub fn iceberg::arrow::ArrowReaderBuilder::with_batch_size(self, batch_size: usi pub fn iceberg::arrow::ArrowReaderBuilder::with_bloom_filter_enabled(self, bloom_filter_enabled: bool) -> Self pub fn iceberg::arrow::ArrowReaderBuilder::with_data_file_concurrency_limit(self, val: usize) -> Self pub fn iceberg::arrow::ArrowReaderBuilder::with_metadata_size_hint(self, metadata_size_hint: usize) -> Self +pub fn iceberg::arrow::ArrowReaderBuilder::with_options(self, options: iceberg::arrow::ArrowReaderOptions) -> Self +pub fn iceberg::arrow::ArrowReaderBuilder::with_parquet_options(self, options: iceberg::arrow::ParquetReadOptions) -> Self pub fn iceberg::arrow::ArrowReaderBuilder::with_range_coalesce_bytes(self, range_coalesce_bytes: u64) -> Self pub fn iceberg::arrow::ArrowReaderBuilder::with_range_fetch_concurrency(self, range_fetch_concurrency: usize) -> Self pub fn iceberg::arrow::ArrowReaderBuilder::with_row_group_filtering_enabled(self, row_group_filtering_enabled: bool) -> Self pub fn iceberg::arrow::ArrowReaderBuilder::with_row_selection_enabled(self, row_selection_enabled: bool) -> Self -impl iceberg::arrow::ArrowReaderBuilder -pub fn iceberg::arrow::ArrowReaderBuilder::with_options(self, options: iceberg::arrow::ArrowReaderOptions) -> Self pub struct iceberg::arrow::ArrowReaderOptions impl core::clone::Clone for iceberg::arrow::ArrowReaderOptions pub fn iceberg::arrow::ArrowReaderOptions::clone(&self) -> iceberg::arrow::ArrowReaderOptions @@ -92,11 +92,28 @@ impl core::fmt::Debug for iceberg::arrow::ArrowReaderOptions pub fn iceberg::arrow::ArrowReaderOptions::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::StructuralPartialEq for iceberg::arrow::ArrowReaderOptions impl iceberg::arrow::ArrowReaderOptions -pub fn iceberg::arrow::ArrowReaderOptions::builder() -> ArrowReaderOptionsBuilder<((), (), (), (), ())> +pub fn iceberg::arrow::ArrowReaderOptions::builder() -> ArrowReaderOptionsBuilder<((), (), (), (), (), ())> impl serde_core::ser::Serialize for iceberg::arrow::ArrowReaderOptions pub fn iceberg::arrow::ArrowReaderOptions::serialize<__S>(&self, __serializer: __S) -> core::result::Result<<__S as serde_core::ser::Serializer>::Ok, <__S as serde_core::ser::Serializer>::Error> where __S: serde_core::ser::Serializer impl<'de> serde_core::de::Deserialize<'de> for iceberg::arrow::ArrowReaderOptions where iceberg::arrow::ArrowReaderOptions: core::default::Default pub fn iceberg::arrow::ArrowReaderOptions::deserialize<__D>(__deserializer: __D) -> core::result::Result::Error> where __D: serde_core::de::Deserializer<'de> +pub struct iceberg::arrow::ParquetReadOptions +impl core::clone::Clone for iceberg::arrow::ParquetReadOptions +pub fn iceberg::arrow::ParquetReadOptions::clone(&self) -> iceberg::arrow::ParquetReadOptions +impl core::cmp::PartialEq for iceberg::arrow::ParquetReadOptions +pub fn iceberg::arrow::ParquetReadOptions::eq(&self, other: &iceberg::arrow::ParquetReadOptions) -> bool +impl core::default::Default for iceberg::arrow::ParquetReadOptions +pub fn iceberg::arrow::ParquetReadOptions::default() -> Self +impl core::fmt::Debug for iceberg::arrow::ParquetReadOptions +pub fn iceberg::arrow::ParquetReadOptions::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::Copy for iceberg::arrow::ParquetReadOptions +impl core::marker::StructuralPartialEq for iceberg::arrow::ParquetReadOptions +impl iceberg::arrow::ParquetReadOptions +pub fn iceberg::arrow::ParquetReadOptions::builder() -> ParquetReadOptionsBuilder<((), (), ())> +impl serde_core::ser::Serialize for iceberg::arrow::ParquetReadOptions +pub fn iceberg::arrow::ParquetReadOptions::serialize<__S>(&self, __serializer: __S) -> core::result::Result<<__S as serde_core::ser::Serializer>::Ok, <__S as serde_core::ser::Serializer>::Error> where __S: serde_core::ser::Serializer +impl<'de> serde_core::de::Deserialize<'de> for iceberg::arrow::ParquetReadOptions where iceberg::arrow::ParquetReadOptions: core::default::Default +pub fn iceberg::arrow::ParquetReadOptions::deserialize<__D>(__deserializer: __D) -> core::result::Result::Error> where __D: serde_core::de::Deserializer<'de> pub struct iceberg::arrow::PartitionValueCalculator impl iceberg::arrow::PartitionValueCalculator pub fn iceberg::arrow::PartitionValueCalculator::calculate(&self, batch: &arrow_array::record_batch::RecordBatch) -> iceberg::Result diff --git a/crates/iceberg/src/arrow/reader/mod.rs b/crates/iceberg/src/arrow/reader/mod.rs index a6aebf37fc..ca2e8da8e2 100644 --- a/crates/iceberg/src/arrow/reader/mod.rs +++ b/crates/iceberg/src/arrow/reader/mod.rs @@ -19,8 +19,6 @@ use crate::arrow::caching_delete_file_loader::CachingDeleteFileLoader; use crate::io::FileIO; -use crate::runtime::Runtime; -use crate::util::available_parallelism; /// Default gap between byte ranges below which they are coalesced into a /// single request. Matches object_store's `OBJECT_STORE_COALESCE_DEFAULT`. @@ -44,130 +42,14 @@ mod projection; mod row_filter; mod row_lineage; pub use file_reader::ArrowFileReader; -pub use options::{ArrowReaderOptions, ArrowReaderOptionsBuilder}; -pub(crate) use parquet_options::ParquetReadOptions; +pub use options::{ArrowReaderBuilder, ArrowReaderOptions, ArrowReaderOptionsBuilder}; +pub use parquet_options::{ParquetReadOptions, ParquetReadOptionsBuilder}; use predicate_visitor::{CollectFieldIdVisitor, PredicateConverter}; use projection::{ add_fallback_field_ids_to_arrow_schema, apply_name_mapping_to_arrow_schema, find_leaf_by_field_id, }; -/// Builder to create ArrowReader -pub struct ArrowReaderBuilder { - batch_size: Option, - file_io: FileIO, - concurrency_limit_data_files: Option, - row_group_filtering_enabled: bool, - row_selection_enabled: bool, - bloom_filter_enabled: bool, - parquet_read_options: ParquetReadOptions, - runtime: Runtime, -} - -impl ArrowReaderBuilder { - /// Create a new ArrowReaderBuilder - pub fn new(file_io: FileIO, runtime: Runtime) -> Self { - ArrowReaderBuilder { - batch_size: None, - file_io, - concurrency_limit_data_files: None, - row_group_filtering_enabled: true, - row_selection_enabled: false, - bloom_filter_enabled: false, - parquet_read_options: ParquetReadOptions::builder().build(), - runtime, - } - } - - /// Sets the max number of in flight data files that are being fetched - pub fn with_data_file_concurrency_limit(mut self, val: usize) -> Self { - self.concurrency_limit_data_files = Some(val); - self - } - - /// Sets the desired size of batches in the response - /// to something other than the default - pub fn with_batch_size(mut self, batch_size: usize) -> Self { - self.batch_size = Some(batch_size); - self - } - - /// Determines whether to enable row group filtering. - pub fn with_row_group_filtering_enabled(mut self, row_group_filtering_enabled: bool) -> Self { - self.row_group_filtering_enabled = row_group_filtering_enabled; - self - } - - /// Determines whether to enable row selection. - pub fn with_row_selection_enabled(mut self, row_selection_enabled: bool) -> Self { - self.row_selection_enabled = row_selection_enabled; - self - } - - /// Determines whether to enable bloom filter-based row group filtering. - /// - /// When enabled, if a read is performed with an equality or IN predicate, - /// the bloom filter for relevant columns in each row group is read and - /// checked. Row groups where the bloom filter proves the value is absent - /// are skipped entirely. - /// - /// Defaults to disabled. Each bloom filter is a separate read, and they are - /// issued serially — one round trip per relevant column per row group, before - /// any data is read. TODO(#3191) - pub fn with_bloom_filter_enabled(mut self, bloom_filter_enabled: bool) -> Self { - self.bloom_filter_enabled = bloom_filter_enabled; - self - } - - /// Provide a hint as to the number of bytes to prefetch for parsing the Parquet metadata - /// - /// This hint can help reduce the number of fetch requests. For more details see the - /// [ParquetMetaDataReader documentation](https://docs.rs/parquet/latest/parquet/file/metadata/struct.ParquetMetaDataReader.html#method.with_prefetch_hint). - pub fn with_metadata_size_hint(mut self, metadata_size_hint: usize) -> Self { - self.parquet_read_options.metadata_size_hint = Some(metadata_size_hint); - self - } - - /// Sets the gap threshold for merging nearby byte ranges into a single request. - /// Ranges with gaps smaller than this value will be coalesced. - /// - /// Defaults to 1 MiB, matching object_store's OBJECT_STORE_COALESCE_DEFAULT. - pub fn with_range_coalesce_bytes(mut self, range_coalesce_bytes: u64) -> Self { - self.parquet_read_options.range_coalesce_bytes = range_coalesce_bytes; - self - } - - /// Sets the maximum number of merged byte ranges to fetch concurrently. - /// - /// Defaults to 10, matching object_store's OBJECT_STORE_COALESCE_PARALLEL. - pub fn with_range_fetch_concurrency(mut self, range_fetch_concurrency: usize) -> Self { - self.parquet_read_options.range_fetch_concurrency = range_fetch_concurrency; - self - } - - /// Build the ArrowReader. - pub fn build(self) -> ArrowReader { - let concurrency_limit_data_files = self - .concurrency_limit_data_files - .unwrap_or_else(|| available_parallelism().get()); - ArrowReader { - batch_size: self.batch_size, - file_io: self.file_io.clone(), - delete_file_loader: CachingDeleteFileLoader::new( - self.file_io.clone(), - concurrency_limit_data_files, - self.runtime.clone(), - ), - concurrency_limit_data_files, - row_group_filtering_enabled: self.row_group_filtering_enabled, - row_selection_enabled: self.row_selection_enabled, - bloom_filter_enabled: self.bloom_filter_enabled, - parquet_read_options: self.parquet_read_options, - runtime: self.runtime.clone(), - } - } -} - /// Reads data from Parquet files #[derive(Clone)] pub struct ArrowReader { diff --git a/crates/iceberg/src/arrow/reader/options.rs b/crates/iceberg/src/arrow/reader/options.rs index df094ef3b9..46f6ced9f6 100644 --- a/crates/iceberg/src/arrow/reader/options.rs +++ b/crates/iceberg/src/arrow/reader/options.rs @@ -18,9 +18,29 @@ use serde::{Deserialize, Serialize}; use typed_builder::TypedBuilder; -use super::ArrowReaderBuilder; +use super::{ArrowReader, ParquetReadOptions}; +use crate::arrow::caching_delete_file_loader::CachingDeleteFileLoader; +use crate::io::FileIO; +use crate::runtime::Runtime; +use crate::util::available_parallelism; /// Configures Arrow reads independently of file planning and local I/O resources. +/// +/// ```no_run +/// # use iceberg::{Result, arrow::{ArrowReaderOptions, ParquetReadOptions}, table::Table}; +/// # async fn read(table: &Table) -> Result<()> { +/// let options = ArrowReaderOptions::builder() +/// .with_batch_size(Some(1024)) +/// .with_parquet_options( +/// ParquetReadOptions::builder() +/// .with_metadata_size_hint(Some(512 * 1024)) +/// .build(), +/// ) +/// .build(); +/// let stream = table.scan().build()?.to_arrow(options).await?; +/// # Ok(()) +/// # } +/// ``` #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, TypedBuilder)] #[serde(default)] #[builder(field_defaults(default, setter(prefix = "with_")))] @@ -36,6 +56,8 @@ pub struct ArrowReaderOptions { row_selection_enabled: bool, /// Whether to use bloom filters for row-group pruning. bloom_filter_enabled: bool, + /// I/O tuning applied when reading Parquet files. + parquet_options: ParquetReadOptions, } impl Default for ArrowReaderOptions { @@ -44,17 +66,122 @@ impl Default for ArrowReaderOptions { } } +/// Builder to create ArrowReader +pub struct ArrowReaderBuilder { + options: ArrowReaderOptions, + file_io: FileIO, + runtime: Runtime, +} + impl ArrowReaderBuilder { - /// Replaces reader settings while preserving local resources and Parquet I/O tuning. - /// Unset batch size and concurrency restore the defaults used at reader construction. + /// Create a new ArrowReaderBuilder + pub fn new(file_io: FileIO, runtime: Runtime) -> Self { + ArrowReaderBuilder { + options: ArrowReaderOptions::default(), + file_io, + runtime, + } + } + + /// Sets the max number of in flight data files that are being fetched + pub fn with_data_file_concurrency_limit(mut self, val: usize) -> Self { + self.options.data_file_concurrency_limit = Some(val); + self + } + + /// Sets the desired size of batches in the response + /// to something other than the default + pub fn with_batch_size(mut self, batch_size: usize) -> Self { + self.options.batch_size = Some(batch_size); + self + } + + /// Determines whether to enable row group filtering. + pub fn with_row_group_filtering_enabled(mut self, row_group_filtering_enabled: bool) -> Self { + self.options.row_group_filtering_enabled = row_group_filtering_enabled; + self + } + + /// Determines whether to enable row selection. + pub fn with_row_selection_enabled(mut self, row_selection_enabled: bool) -> Self { + self.options.row_selection_enabled = row_selection_enabled; + self + } + + /// Determines whether to enable bloom filter-based row group filtering. + /// + /// When enabled, if a read is performed with an equality or IN predicate, + /// the bloom filter for relevant columns in each row group is read and + /// checked. Row groups where the bloom filter proves the value is absent + /// are skipped entirely. + /// + /// Defaults to disabled. Each bloom filter is a separate read, and they are + /// issued serially — one round trip per relevant column per row group, before + /// any data is read. TODO(#3191) + pub fn with_bloom_filter_enabled(mut self, bloom_filter_enabled: bool) -> Self { + self.options.bloom_filter_enabled = bloom_filter_enabled; + self + } + + /// Provide a hint as to the number of bytes to prefetch for parsing the Parquet metadata + /// + /// This hint can help reduce the number of fetch requests. For more details see the + /// [ParquetMetaDataReader documentation](https://docs.rs/parquet/latest/parquet/file/metadata/struct.ParquetMetaDataReader.html#method.with_prefetch_hint). + pub fn with_metadata_size_hint(mut self, metadata_size_hint: usize) -> Self { + self.options.parquet_options.metadata_size_hint = Some(metadata_size_hint); + self + } + + /// Sets the gap threshold for merging nearby byte ranges into a single request. + /// Ranges with gaps smaller than this value will be coalesced. + /// + /// Defaults to 1 MiB, matching object_store's OBJECT_STORE_COALESCE_DEFAULT. + pub fn with_range_coalesce_bytes(mut self, range_coalesce_bytes: u64) -> Self { + self.options.parquet_options.range_coalesce_bytes = range_coalesce_bytes; + self + } + + /// Sets the maximum number of merged byte ranges to fetch concurrently. + /// + /// Defaults to 10, matching object_store's OBJECT_STORE_COALESCE_PARALLEL. + pub fn with_range_fetch_concurrency(mut self, range_fetch_concurrency: usize) -> Self { + self.options.parquet_options.range_fetch_concurrency = range_fetch_concurrency; + self + } + + /// Replaces all reader settings while preserving local I/O and runtime resources. pub fn with_options(mut self, options: ArrowReaderOptions) -> Self { - self.batch_size = options.batch_size; - self.concurrency_limit_data_files = options.data_file_concurrency_limit; - self.row_group_filtering_enabled = options.row_group_filtering_enabled; - self.row_selection_enabled = options.row_selection_enabled; - self.bloom_filter_enabled = options.bloom_filter_enabled; + self.options = options; + self + } + + /// Replaces Parquet I/O settings while preserving other reader settings. + pub fn with_parquet_options(mut self, options: ParquetReadOptions) -> Self { + self.options.parquet_options = options; self } + + /// Build the ArrowReader. + pub fn build(self) -> ArrowReader { + let concurrency_limit_data_files = self + .options + .data_file_concurrency_limit + .unwrap_or_else(|| available_parallelism().get()); + ArrowReader { + batch_size: self.options.batch_size, + file_io: self.file_io.clone(), + delete_file_loader: CachingDeleteFileLoader::new( + self.file_io.clone(), + concurrency_limit_data_files, + self.runtime.clone(), + ), + concurrency_limit_data_files, + row_group_filtering_enabled: self.options.row_group_filtering_enabled, + row_selection_enabled: self.options.row_selection_enabled, + bloom_filter_enabled: self.options.bloom_filter_enabled, + parquet_read_options: self.options.parquet_options, + } + } } #[cfg(test)] @@ -82,6 +209,52 @@ mod tests { ); } + #[test] + fn partial_parquet_options_preserve_defaults() { + let options: ArrowReaderOptions = serde_json::from_str( + r#"{"parquet_options":{"range_fetch_concurrency":2,"future_option":true}}"#, + ) + .unwrap(); + assert_eq!( + options, + ArrowReaderOptions::builder() + .with_parquet_options( + ParquetReadOptions::builder() + .with_range_fetch_concurrency(2) + .build() + ) + .build() + ); + } + + #[test] + fn parquet_override_preserves_reader_settings_and_allows_individual_overrides() { + let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) + .with_options( + ArrowReaderOptions::builder() + .with_batch_size(Some(17)) + .build(), + ) + .with_parquet_options( + ParquetReadOptions::builder() + .with_metadata_size_hint(Some(4096)) + .with_range_fetch_concurrency(2) + .build(), + ) + .with_range_fetch_concurrency(3) + .build(); + assert_eq!( + (reader.batch_size, reader.parquet_read_options), + ( + Some(17), + ParquetReadOptions::builder() + .with_metadata_size_hint(Some(4096)) + .with_range_fetch_concurrency(3) + .build() + ) + ); + } + #[test] fn options_restore_local_defaults_or_apply_explicit_settings() { for explicit in [false, true] { @@ -92,6 +265,13 @@ mod tests { .with_row_group_filtering_enabled(false) .with_row_selection_enabled(true) .with_bloom_filter_enabled(true) + .with_parquet_options( + ParquetReadOptions::builder() + .with_metadata_size_hint(Some(4096)) + .with_range_coalesce_bytes(8192) + .with_range_fetch_concurrency(2) + .build(), + ) .build() } else { ArrowReaderOptions::default() @@ -125,7 +305,11 @@ mod tests { reader.parquet_read_options.range_coalesce_bytes(), reader.parquet_read_options.range_fetch_concurrency() ), - (Some(4096), 8192, 2) + if explicit { + (Some(4096), 8192, 2) + } else { + (Some(512 * 1024), 1024 * 1024, 10) + } ); } } diff --git a/crates/iceberg/src/arrow/reader/parquet_options.rs b/crates/iceberg/src/arrow/reader/parquet_options.rs index ae6a3ed18e..05081b6061 100644 --- a/crates/iceberg/src/arrow/reader/parquet_options.rs +++ b/crates/iceberg/src/arrow/reader/parquet_options.rs @@ -17,6 +17,7 @@ //! Tunables for Parquet file I/O used by `ArrowReader`. +use serde::{Deserialize, Serialize}; use typed_builder::TypedBuilder; use super::{ @@ -24,9 +25,10 @@ use super::{ }; /// Options for tuning Parquet file I/O. -#[derive(Clone, Copy, Debug, TypedBuilder)] +#[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize, TypedBuilder)] +#[serde(default)] #[builder(field_defaults(setter(prefix = "with_")))] -pub(crate) struct ParquetReadOptions { +pub struct ParquetReadOptions { /// Number of bytes to prefetch for parsing the Parquet metadata. /// /// This hint can help reduce the number of fetch requests. For more details see the @@ -47,16 +49,25 @@ pub(crate) struct ParquetReadOptions { #[builder(default = DEFAULT_RANGE_FETCH_CONCURRENCY)] pub(crate) range_fetch_concurrency: usize, /// Whether to preload the column index when reading Parquet metadata. - #[builder(default = true)] + #[builder(default = true, setter(skip))] + #[serde(skip)] pub(crate) preload_column_index: bool, /// Whether to preload the offset index when reading Parquet metadata. - #[builder(default = true)] + #[builder(default = true, setter(skip))] + #[serde(skip)] pub(crate) preload_offset_index: bool, /// Whether to preload the page index when reading Parquet metadata. - #[builder(default = false)] + #[builder(default = false, setter(skip))] + #[serde(skip)] pub(crate) preload_page_index: bool, } +impl Default for ParquetReadOptions { + fn default() -> Self { + Self::builder().build() + } +} + impl ParquetReadOptions { pub(crate) fn metadata_size_hint(&self) -> Option { self.metadata_size_hint diff --git a/crates/iceberg/src/scan/mod.rs b/crates/iceberg/src/scan/mod.rs index 803f3f4ab7..9d11d00882 100644 --- a/crates/iceberg/src/scan/mod.rs +++ b/crates/iceberg/src/scan/mod.rs @@ -651,7 +651,7 @@ mod tests { use uuid::Uuid; use super::resolve_field_id; - use crate::arrow::{ArrowReaderBuilder, ArrowReaderOptions}; + use crate::arrow::{ArrowReaderBuilder, ArrowReaderOptions, ParquetReadOptions}; use crate::expr::{BoundPredicate, Reference}; use crate::io::FileIO; use crate::metadata_columns::{ @@ -1901,6 +1901,12 @@ mod tests { let options = ArrowReaderOptions::builder() .with_batch_size(Some(17)) .with_data_file_concurrency_limit(Some(1)) + .with_parquet_options( + ParquetReadOptions::builder() + .with_metadata_size_hint(Some(4096)) + .with_range_fetch_concurrency(2) + .build(), + ) .with_row_selection_enabled(true) .build(); let expected: Vec<_> = scan @@ -1924,6 +1930,7 @@ mod tests { "batch_size": 17, "data_file_concurrency_limit": 1, "row_group_filtering_enabled": true, "row_selection_enabled": true, "bloom_filter_enabled": false, + "parquet_options": { "metadata_size_hint": 4096, "range_coalesce_bytes": 1048576, "range_fetch_concurrency": 2 }, }) ); let options: ArrowReaderOptions = serde_json::from_value(serialized).unwrap(); From bfa9ee0ece5275fabdafc8797280fac61a83a3dc Mon Sep 17 00:00:00 2001 From: "alex.bianchi" Date: Tue, 27 Oct 2026 16:03:53 -0400 Subject: [PATCH 04/15] refactor(arrow): refine reader options API after review --- crates/iceberg/README.md | 2 +- crates/iceberg/public-api.txt | 11 + crates/iceberg/src/arrow/reader/mod.rs | 134 ++++++++++- crates/iceberg/src/arrow/reader/options.rs | 218 ++++++++++-------- .../src/arrow/reader/parquet_options.rs | 95 -------- crates/iceberg/src/arrow/reader/pipeline.rs | 2 +- crates/iceberg/src/scan/mod.rs | 2 +- 7 files changed, 265 insertions(+), 199 deletions(-) delete mode 100644 crates/iceberg/src/arrow/reader/parquet_options.rs diff --git a/crates/iceberg/README.md b/crates/iceberg/README.md index f6688c0a52..0cac1a4cd1 100644 --- a/crates/iceberg/README.md +++ b/crates/iceberg/README.md @@ -75,7 +75,7 @@ let options = iceberg::arrow::ArrowReaderOptions::builder() .with_row_selection_enabled(true) .with_parquet_options( iceberg::arrow::ParquetReadOptions::builder() - .with_metadata_size_hint(Some(512 * 1024)) + .with_metadata_size_hint(512 * 1024) .build(), ) .build(); diff --git a/crates/iceberg/public-api.txt b/crates/iceberg/public-api.txt index 35345f3eae..818c2069c8 100644 --- a/crates/iceberg/public-api.txt +++ b/crates/iceberg/public-api.txt @@ -82,6 +82,13 @@ pub fn iceberg::arrow::ArrowReaderBuilder::with_range_fetch_concurrency(self, ra pub fn iceberg::arrow::ArrowReaderBuilder::with_row_group_filtering_enabled(self, row_group_filtering_enabled: bool) -> Self pub fn iceberg::arrow::ArrowReaderBuilder::with_row_selection_enabled(self, row_selection_enabled: bool) -> Self pub struct iceberg::arrow::ArrowReaderOptions +impl iceberg::arrow::ArrowReaderOptions +pub fn iceberg::arrow::ArrowReaderOptions::batch_size(&self) -> core::option::Option +pub fn iceberg::arrow::ArrowReaderOptions::bloom_filter_enabled(&self) -> bool +pub fn iceberg::arrow::ArrowReaderOptions::data_file_concurrency_limit(&self) -> core::option::Option +pub fn iceberg::arrow::ArrowReaderOptions::parquet_options(&self) -> &iceberg::arrow::ParquetReadOptions +pub fn iceberg::arrow::ArrowReaderOptions::row_group_filtering_enabled(&self) -> bool +pub fn iceberg::arrow::ArrowReaderOptions::row_selection_enabled(&self) -> bool impl core::clone::Clone for iceberg::arrow::ArrowReaderOptions pub fn iceberg::arrow::ArrowReaderOptions::clone(&self) -> iceberg::arrow::ArrowReaderOptions impl core::cmp::PartialEq for iceberg::arrow::ArrowReaderOptions @@ -98,6 +105,10 @@ pub fn iceberg::arrow::ArrowReaderOptions::serialize<__S>(&self, __serializer: _ impl<'de> serde_core::de::Deserialize<'de> for iceberg::arrow::ArrowReaderOptions where iceberg::arrow::ArrowReaderOptions: core::default::Default pub fn iceberg::arrow::ArrowReaderOptions::deserialize<__D>(__deserializer: __D) -> core::result::Result::Error> where __D: serde_core::de::Deserializer<'de> pub struct iceberg::arrow::ParquetReadOptions +impl iceberg::arrow::ParquetReadOptions +pub fn iceberg::arrow::ParquetReadOptions::metadata_size_hint(&self) -> core::option::Option +pub fn iceberg::arrow::ParquetReadOptions::range_coalesce_bytes(&self) -> u64 +pub fn iceberg::arrow::ParquetReadOptions::range_fetch_concurrency(&self) -> usize impl core::clone::Clone for iceberg::arrow::ParquetReadOptions pub fn iceberg::arrow::ParquetReadOptions::clone(&self) -> iceberg::arrow::ParquetReadOptions impl core::cmp::PartialEq for iceberg::arrow::ParquetReadOptions diff --git a/crates/iceberg/src/arrow/reader/mod.rs b/crates/iceberg/src/arrow/reader/mod.rs index ca2e8da8e2..59a9594dcd 100644 --- a/crates/iceberg/src/arrow/reader/mod.rs +++ b/crates/iceberg/src/arrow/reader/mod.rs @@ -19,6 +19,8 @@ use crate::arrow::caching_delete_file_loader::CachingDeleteFileLoader; use crate::io::FileIO; +use crate::runtime::Runtime; +use crate::util::available_parallelism; /// Default gap between byte ranges below which they are coalesced into a /// single request. Matches object_store's `OBJECT_STORE_COALESCE_DEFAULT`. @@ -34,7 +36,6 @@ const DEFAULT_METADATA_SIZE_HINT: usize = 512 * 1024; mod file_reader; mod options; -mod parquet_options; mod pipeline; mod positional_deletes; mod predicate_visitor; @@ -42,14 +43,141 @@ mod projection; mod row_filter; mod row_lineage; pub use file_reader::ArrowFileReader; -pub use options::{ArrowReaderBuilder, ArrowReaderOptions, ArrowReaderOptionsBuilder}; -pub use parquet_options::{ParquetReadOptions, ParquetReadOptionsBuilder}; +pub use options::{ + ArrowReaderOptions, ArrowReaderOptionsBuilder, ParquetReadOptions, ParquetReadOptionsBuilder, +}; use predicate_visitor::{CollectFieldIdVisitor, PredicateConverter}; use projection::{ add_fallback_field_ids_to_arrow_schema, apply_name_mapping_to_arrow_schema, find_leaf_by_field_id, }; +/// Builder to create ArrowReader +pub struct ArrowReaderBuilder { + options: ArrowReaderOptions, + file_io: FileIO, + runtime: Runtime, +} + +impl ArrowReaderBuilder { + /// Create a new ArrowReaderBuilder + pub fn new(file_io: FileIO, runtime: Runtime) -> Self { + ArrowReaderBuilder { + options: ArrowReaderOptions::default(), + file_io, + runtime, + } + } + + /// Sets the max number of in flight data files that are being fetched + pub fn with_data_file_concurrency_limit(mut self, val: usize) -> Self { + self.options.set_data_file_concurrency_limit(Some(val)); + self + } + + /// Sets the desired size of batches in the response + /// to something other than the default + pub fn with_batch_size(mut self, batch_size: usize) -> Self { + self.options.set_batch_size(Some(batch_size)); + self + } + + /// Determines whether to enable row group filtering. + pub fn with_row_group_filtering_enabled(mut self, row_group_filtering_enabled: bool) -> Self { + self.options + .set_row_group_filtering_enabled(row_group_filtering_enabled); + self + } + + /// Determines whether to enable row selection. + pub fn with_row_selection_enabled(mut self, row_selection_enabled: bool) -> Self { + self.options + .set_row_selection_enabled(row_selection_enabled); + self + } + + /// Determines whether to enable bloom filter-based row group filtering. + /// + /// When enabled, if a read is performed with an equality or IN predicate, + /// the bloom filter for relevant columns in each row group is read and + /// checked. Row groups where the bloom filter proves the value is absent + /// are skipped entirely. + /// + /// Defaults to disabled. Each bloom filter is a separate read, and they are + /// issued serially — one round trip per relevant column per row group, before + /// any data is read. TODO(#3191) + pub fn with_bloom_filter_enabled(mut self, bloom_filter_enabled: bool) -> Self { + self.options.set_bloom_filter_enabled(bloom_filter_enabled); + self + } + + /// Provide a hint as to the number of bytes to prefetch for parsing the Parquet metadata + /// + /// This hint can help reduce the number of fetch requests. For more details see the + /// [ParquetMetaDataReader documentation](https://docs.rs/parquet/latest/parquet/file/metadata/struct.ParquetMetaDataReader.html#method.with_prefetch_hint). + pub fn with_metadata_size_hint(mut self, metadata_size_hint: usize) -> Self { + self.options + .parquet_options_mut() + .set_metadata_size_hint(Some(metadata_size_hint)); + self + } + + /// Sets the gap threshold for merging nearby byte ranges into a single request. + /// Ranges with gaps smaller than this value will be coalesced. + /// + /// Defaults to 1 MiB, matching object_store's OBJECT_STORE_COALESCE_DEFAULT. + pub fn with_range_coalesce_bytes(mut self, range_coalesce_bytes: u64) -> Self { + self.options + .parquet_options_mut() + .set_range_coalesce_bytes(range_coalesce_bytes); + self + } + + /// Sets the maximum number of merged byte ranges to fetch concurrently. + /// + /// Defaults to 10, matching object_store's OBJECT_STORE_COALESCE_PARALLEL. + pub fn with_range_fetch_concurrency(mut self, range_fetch_concurrency: usize) -> Self { + self.options + .parquet_options_mut() + .set_range_fetch_concurrency(range_fetch_concurrency); + self + } + + /// Replaces all reader settings while preserving local I/O and runtime resources. + pub fn with_options(mut self, options: ArrowReaderOptions) -> Self { + self.options = options; + self + } + + /// Replaces Parquet I/O settings while preserving other reader settings. + pub fn with_parquet_options(mut self, options: ParquetReadOptions) -> Self { + self.options.set_parquet_options(options); + self + } + + /// Build the ArrowReader. + pub fn build(self) -> ArrowReader { + let concurrency_limit_data_files = self + .options + .data_file_concurrency_limit() + .unwrap_or_else(|| available_parallelism().get()); + ArrowReader { + batch_size: self.options.batch_size(), + file_io: self.file_io.clone(), + delete_file_loader: CachingDeleteFileLoader::new( + self.file_io.clone(), + concurrency_limit_data_files, + self.runtime.clone(), + ), + concurrency_limit_data_files, + row_group_filtering_enabled: self.options.row_group_filtering_enabled(), + row_selection_enabled: self.options.row_selection_enabled(), + bloom_filter_enabled: self.options.bloom_filter_enabled(), + parquet_read_options: *self.options.parquet_options(), + } + } +} + /// Reads data from Parquet files #[derive(Clone)] pub struct ArrowReader { diff --git a/crates/iceberg/src/arrow/reader/options.rs b/crates/iceberg/src/arrow/reader/options.rs index 46f6ced9f6..f93dc62f8b 100644 --- a/crates/iceberg/src/arrow/reader/options.rs +++ b/crates/iceberg/src/arrow/reader/options.rs @@ -18,11 +18,9 @@ use serde::{Deserialize, Serialize}; use typed_builder::TypedBuilder; -use super::{ArrowReader, ParquetReadOptions}; -use crate::arrow::caching_delete_file_loader::CachingDeleteFileLoader; -use crate::io::FileIO; -use crate::runtime::Runtime; -use crate::util::available_parallelism; +use super::{ + DEFAULT_METADATA_SIZE_HINT, DEFAULT_RANGE_COALESCE_BYTES, DEFAULT_RANGE_FETCH_CONCURRENCY, +}; /// Configures Arrow reads independently of file planning and local I/O resources. /// @@ -33,7 +31,7 @@ use crate::util::available_parallelism; /// .with_batch_size(Some(1024)) /// .with_parquet_options( /// ParquetReadOptions::builder() -/// .with_metadata_size_hint(Some(512 * 1024)) +/// .with_metadata_size_hint(512 * 1024) /// .build(), /// ) /// .build(); @@ -66,127 +64,151 @@ impl Default for ArrowReaderOptions { } } -/// Builder to create ArrowReader -pub struct ArrowReaderBuilder { - options: ArrowReaderOptions, - file_io: FileIO, - runtime: Runtime, -} +impl ArrowReaderOptions { + /// Returns the configured batch size. + pub fn batch_size(&self) -> Option { + self.batch_size + } -impl ArrowReaderBuilder { - /// Create a new ArrowReaderBuilder - pub fn new(file_io: FileIO, runtime: Runtime) -> Self { - ArrowReaderBuilder { - options: ArrowReaderOptions::default(), - file_io, - runtime, - } + pub(super) fn parquet_options_mut(&mut self) -> &mut ParquetReadOptions { + &mut self.parquet_options } - /// Sets the max number of in flight data files that are being fetched - pub fn with_data_file_concurrency_limit(mut self, val: usize) -> Self { - self.options.data_file_concurrency_limit = Some(val); - self + pub(super) fn set_batch_size(&mut self, value: Option) { + self.batch_size = value; + } + /// Returns the configured data file concurrency limit. + pub fn data_file_concurrency_limit(&self) -> Option { + self.data_file_concurrency_limit } - /// Sets the desired size of batches in the response - /// to something other than the default - pub fn with_batch_size(mut self, batch_size: usize) -> Self { - self.options.batch_size = Some(batch_size); - self + pub(super) fn set_data_file_concurrency_limit(&mut self, value: Option) { + self.data_file_concurrency_limit = value; + } + /// Returns whether row-group statistics filtering is enabled. + pub fn row_group_filtering_enabled(&self) -> bool { + self.row_group_filtering_enabled } - /// Determines whether to enable row group filtering. - pub fn with_row_group_filtering_enabled(mut self, row_group_filtering_enabled: bool) -> Self { - self.options.row_group_filtering_enabled = row_group_filtering_enabled; - self + pub(super) fn set_row_group_filtering_enabled(&mut self, value: bool) { + self.row_group_filtering_enabled = value; + } + /// Returns whether page-index row selection is enabled. + pub fn row_selection_enabled(&self) -> bool { + self.row_selection_enabled } - /// Determines whether to enable row selection. - pub fn with_row_selection_enabled(mut self, row_selection_enabled: bool) -> Self { - self.options.row_selection_enabled = row_selection_enabled; - self + pub(super) fn set_row_selection_enabled(&mut self, value: bool) { + self.row_selection_enabled = value; + } + /// Returns whether bloom-filter pruning is enabled. + pub fn bloom_filter_enabled(&self) -> bool { + self.bloom_filter_enabled } - /// Determines whether to enable bloom filter-based row group filtering. - /// - /// When enabled, if a read is performed with an equality or IN predicate, - /// the bloom filter for relevant columns in each row group is read and - /// checked. Row groups where the bloom filter proves the value is absent - /// are skipped entirely. - /// - /// Defaults to disabled. Each bloom filter is a separate read, and they are - /// issued serially — one round trip per relevant column per row group, before - /// any data is read. TODO(#3191) - pub fn with_bloom_filter_enabled(mut self, bloom_filter_enabled: bool) -> Self { - self.options.bloom_filter_enabled = bloom_filter_enabled; - self + pub(super) fn set_bloom_filter_enabled(&mut self, value: bool) { + self.bloom_filter_enabled = value; } + /// Returns the configured parquet options. + pub fn parquet_options(&self) -> &ParquetReadOptions { + &self.parquet_options + } + + pub(super) fn set_parquet_options(&mut self, value: ParquetReadOptions) { + self.parquet_options = value; + } +} - /// Provide a hint as to the number of bytes to prefetch for parsing the Parquet metadata +/// Options for tuning Parquet file I/O. +#[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize, TypedBuilder)] +#[serde(default)] +#[builder(field_defaults(setter(prefix = "with_")))] +pub struct ParquetReadOptions { + /// Number of bytes to prefetch for parsing the Parquet metadata. /// /// This hint can help reduce the number of fetch requests. For more details see the /// [ParquetMetaDataReader documentation](https://docs.rs/parquet/latest/parquet/file/metadata/struct.ParquetMetaDataReader.html#method.with_prefetch_hint). - pub fn with_metadata_size_hint(mut self, metadata_size_hint: usize) -> Self { - self.options.parquet_options.metadata_size_hint = Some(metadata_size_hint); - self - } - - /// Sets the gap threshold for merging nearby byte ranges into a single request. + /// + /// Defaults to 512 KiB, matching DataFusion's default `ParquetOptions::metadata_size_hint`. + #[builder(default = Some(DEFAULT_METADATA_SIZE_HINT), setter(strip_option))] + metadata_size_hint: Option, + /// Gap threshold for merging nearby byte ranges into a single request. /// Ranges with gaps smaller than this value will be coalesced. /// - /// Defaults to 1 MiB, matching object_store's OBJECT_STORE_COALESCE_DEFAULT. - pub fn with_range_coalesce_bytes(mut self, range_coalesce_bytes: u64) -> Self { - self.options.parquet_options.range_coalesce_bytes = range_coalesce_bytes; - self + /// Defaults to 1 MiB, matching object_store's `OBJECT_STORE_COALESCE_DEFAULT`. + #[builder(default = DEFAULT_RANGE_COALESCE_BYTES)] + range_coalesce_bytes: u64, + /// Maximum number of merged byte ranges to fetch concurrently. + /// + /// Defaults to 10, matching object_store's `OBJECT_STORE_COALESCE_PARALLEL`. + #[builder(default = DEFAULT_RANGE_FETCH_CONCURRENCY)] + range_fetch_concurrency: usize, + /// Whether to preload the column index when reading Parquet metadata. + #[builder(default = true, setter(skip))] + #[serde(skip)] + preload_column_index: bool, + /// Whether to preload the offset index when reading Parquet metadata. + #[builder(default = true, setter(skip))] + #[serde(skip)] + preload_offset_index: bool, + /// Whether to preload the page index when reading Parquet metadata. + #[builder(default = false, setter(skip))] + #[serde(skip)] + preload_page_index: bool, +} + +impl Default for ParquetReadOptions { + fn default() -> Self { + Self::builder().build() } +} - /// Sets the maximum number of merged byte ranges to fetch concurrently. - /// - /// Defaults to 10, matching object_store's OBJECT_STORE_COALESCE_PARALLEL. - pub fn with_range_fetch_concurrency(mut self, range_fetch_concurrency: usize) -> Self { - self.options.parquet_options.range_fetch_concurrency = range_fetch_concurrency; - self +impl ParquetReadOptions { + pub(super) fn set_metadata_size_hint(&mut self, value: Option) { + self.metadata_size_hint = value; + } + pub(super) fn set_range_coalesce_bytes(&mut self, value: u64) { + self.range_coalesce_bytes = value; + } + pub(super) fn set_range_fetch_concurrency(&mut self, value: usize) { + self.range_fetch_concurrency = value; + } + pub(super) fn set_preload_page_index(&mut self, value: bool) { + self.preload_page_index = value; } - /// Replaces all reader settings while preserving local I/O and runtime resources. - pub fn with_options(mut self, options: ArrowReaderOptions) -> Self { - self.options = options; - self + /// Returns the configured metadata size hint. + pub fn metadata_size_hint(&self) -> Option { + self.metadata_size_hint } - /// Replaces Parquet I/O settings while preserving other reader settings. - pub fn with_parquet_options(mut self, options: ParquetReadOptions) -> Self { - self.options.parquet_options = options; - self + /// Returns the byte-gap threshold for coalescing read requests. + pub fn range_coalesce_bytes(&self) -> u64 { + self.range_coalesce_bytes } - /// Build the ArrowReader. - pub fn build(self) -> ArrowReader { - let concurrency_limit_data_files = self - .options - .data_file_concurrency_limit - .unwrap_or_else(|| available_parallelism().get()); - ArrowReader { - batch_size: self.options.batch_size, - file_io: self.file_io.clone(), - delete_file_loader: CachingDeleteFileLoader::new( - self.file_io.clone(), - concurrency_limit_data_files, - self.runtime.clone(), - ), - concurrency_limit_data_files, - row_group_filtering_enabled: self.options.row_group_filtering_enabled, - row_selection_enabled: self.options.row_selection_enabled, - bloom_filter_enabled: self.options.bloom_filter_enabled, - parquet_read_options: self.options.parquet_options, - } + /// Returns the maximum number of concurrent range requests. + pub fn range_fetch_concurrency(&self) -> usize { + self.range_fetch_concurrency + } + + pub(super) fn preload_column_index(&self) -> bool { + self.preload_column_index + } + + pub(super) fn preload_offset_index(&self) -> bool { + self.preload_offset_index + } + + pub(super) fn preload_page_index(&self) -> bool { + self.preload_page_index } } #[cfg(test)] mod tests { use super::*; + use crate::arrow::ArrowReaderBuilder; use crate::io::FileIO; use crate::test_utils::test_runtime; use crate::util::available_parallelism; @@ -237,7 +259,7 @@ mod tests { ) .with_parquet_options( ParquetReadOptions::builder() - .with_metadata_size_hint(Some(4096)) + .with_metadata_size_hint(4096) .with_range_fetch_concurrency(2) .build(), ) @@ -248,7 +270,7 @@ mod tests { ( Some(17), ParquetReadOptions::builder() - .with_metadata_size_hint(Some(4096)) + .with_metadata_size_hint(4096) .with_range_fetch_concurrency(3) .build() ) @@ -267,7 +289,7 @@ mod tests { .with_bloom_filter_enabled(true) .with_parquet_options( ParquetReadOptions::builder() - .with_metadata_size_hint(Some(4096)) + .with_metadata_size_hint(4096) .with_range_coalesce_bytes(8192) .with_range_fetch_concurrency(2) .build(), diff --git a/crates/iceberg/src/arrow/reader/parquet_options.rs b/crates/iceberg/src/arrow/reader/parquet_options.rs deleted file mode 100644 index 05081b6061..0000000000 --- a/crates/iceberg/src/arrow/reader/parquet_options.rs +++ /dev/null @@ -1,95 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -//! Tunables for Parquet file I/O used by `ArrowReader`. - -use serde::{Deserialize, Serialize}; -use typed_builder::TypedBuilder; - -use super::{ - DEFAULT_METADATA_SIZE_HINT, DEFAULT_RANGE_COALESCE_BYTES, DEFAULT_RANGE_FETCH_CONCURRENCY, -}; - -/// Options for tuning Parquet file I/O. -#[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize, TypedBuilder)] -#[serde(default)] -#[builder(field_defaults(setter(prefix = "with_")))] -pub struct ParquetReadOptions { - /// Number of bytes to prefetch for parsing the Parquet metadata. - /// - /// This hint can help reduce the number of fetch requests. For more details see the - /// [ParquetMetaDataReader documentation](https://docs.rs/parquet/latest/parquet/file/metadata/struct.ParquetMetaDataReader.html#method.with_prefetch_hint). - /// - /// Defaults to 512 KiB, matching DataFusion's default `ParquetOptions::metadata_size_hint`. - #[builder(default = Some(DEFAULT_METADATA_SIZE_HINT))] - pub(crate) metadata_size_hint: Option, - /// Gap threshold for merging nearby byte ranges into a single request. - /// Ranges with gaps smaller than this value will be coalesced. - /// - /// Defaults to 1 MiB, matching object_store's `OBJECT_STORE_COALESCE_DEFAULT`. - #[builder(default = DEFAULT_RANGE_COALESCE_BYTES)] - pub(crate) range_coalesce_bytes: u64, - /// Maximum number of merged byte ranges to fetch concurrently. - /// - /// Defaults to 10, matching object_store's `OBJECT_STORE_COALESCE_PARALLEL`. - #[builder(default = DEFAULT_RANGE_FETCH_CONCURRENCY)] - pub(crate) range_fetch_concurrency: usize, - /// Whether to preload the column index when reading Parquet metadata. - #[builder(default = true, setter(skip))] - #[serde(skip)] - pub(crate) preload_column_index: bool, - /// Whether to preload the offset index when reading Parquet metadata. - #[builder(default = true, setter(skip))] - #[serde(skip)] - pub(crate) preload_offset_index: bool, - /// Whether to preload the page index when reading Parquet metadata. - #[builder(default = false, setter(skip))] - #[serde(skip)] - pub(crate) preload_page_index: bool, -} - -impl Default for ParquetReadOptions { - fn default() -> Self { - Self::builder().build() - } -} - -impl ParquetReadOptions { - pub(crate) fn metadata_size_hint(&self) -> Option { - self.metadata_size_hint - } - - pub(crate) fn range_coalesce_bytes(&self) -> u64 { - self.range_coalesce_bytes - } - - pub(crate) fn range_fetch_concurrency(&self) -> usize { - self.range_fetch_concurrency - } - - pub(crate) fn preload_column_index(&self) -> bool { - self.preload_column_index - } - - pub(crate) fn preload_offset_index(&self) -> bool { - self.preload_offset_index - } - - pub(crate) fn preload_page_index(&self) -> bool { - self.preload_page_index - } -} diff --git a/crates/iceberg/src/arrow/reader/pipeline.rs b/crates/iceberg/src/arrow/reader/pipeline.rs index 1ddee493c4..2206e3289d 100644 --- a/crates/iceberg/src/arrow/reader/pipeline.rs +++ b/crates/iceberg/src/arrow/reader/pipeline.rs @@ -154,7 +154,7 @@ impl FileScanTaskReader { let should_load_page_index = (self.row_selection_enabled && task.predicate().is_some()) || !task.deletes().is_empty(); let mut parquet_read_options = self.parquet_read_options; - parquet_read_options.preload_page_index = should_load_page_index; + parquet_read_options.set_preload_page_index(should_load_page_index); let delete_filter_rx = self .delete_file_loader diff --git a/crates/iceberg/src/scan/mod.rs b/crates/iceberg/src/scan/mod.rs index 9d11d00882..5f57e5f128 100644 --- a/crates/iceberg/src/scan/mod.rs +++ b/crates/iceberg/src/scan/mod.rs @@ -1903,7 +1903,7 @@ mod tests { .with_data_file_concurrency_limit(Some(1)) .with_parquet_options( ParquetReadOptions::builder() - .with_metadata_size_hint(Some(4096)) + .with_metadata_size_hint(4096) .with_range_fetch_concurrency(2) .build(), ) From 131bca9a2ce4f7c0de6531630b2b45bfc5db9830 Mon Sep 17 00:00:00 2001 From: "alex.bianchi" Date: Wed, 28 Oct 2026 19:01:45 -0400 Subject: [PATCH 05/15] refactor(arrow): address reader configuration review feedback --- crates/iceberg/README.md | 9 +- crates/iceberg/public-api.txt | 17 +- .../iceberg/src/arrow/delete_file_loader.rs | 1 + .../iceberg/src/arrow/reader/file_reader.rs | 21 +-- crates/iceberg/src/arrow/reader/mod.rs | 84 +-------- crates/iceberg/src/arrow/reader/options.rs | 170 ++++++++---------- crates/iceberg/src/arrow/reader/pipeline.rs | 36 +++- crates/iceberg/src/arrow/reader/projection.rs | 26 ++- crates/iceberg/src/arrow/reader/row_filter.rs | 48 ++++- crates/iceberg/src/cow_rewrite/mod.rs | 7 +- crates/iceberg/src/scan/mod.rs | 11 +- 11 files changed, 198 insertions(+), 232 deletions(-) diff --git a/crates/iceberg/README.md b/crates/iceberg/README.md index 0cac1a4cd1..84edca3dbc 100644 --- a/crates/iceberg/README.md +++ b/crates/iceberg/README.md @@ -71,7 +71,7 @@ Pass reader settings separately from file planning: ```rust let options = iceberg::arrow::ArrowReaderOptions::builder() - .with_batch_size(Some(1024)) + .with_batch_size(1024) .with_row_selection_enabled(true) .with_parquet_options( iceberg::arrow::ParquetReadOptions::builder() @@ -86,13 +86,14 @@ When upgrading, move reader-setting calls from `TableScanBuilder` to `ArrowReaderOptions::builder()` and pass the result to `to_arrow(options)`. Use `to_arrow(Default::default())` for default settings. Scan `with_concurrency_limit()` controls manifest planning only; set -`with_data_file_concurrency_limit(Some(limit))` on the reader options to also +`with_data_file_concurrency_limit(limit)` on the reader options to also limit data-file reads. An unset data-file limit uses execution-host parallelism. Locally constructed readers accept the same options through `ArrowReaderBuilder::with_options(options)`, including Parquet I/O tuning. -This replaces the complete configuration; subsequent convenience setters override -individual settings. `with_parquet_options()` replaces only the nested Parquet +This replaces the complete configuration. Configure individual settings through +the options builders; per-field setters on `ArrowReaderBuilder` have been removed. +`with_parquet_options()` replaces only the nested Parquet configuration. ## Storage Backends diff --git a/crates/iceberg/public-api.txt b/crates/iceberg/public-api.txt index 818c2069c8..1a6e23e2b3 100644 --- a/crates/iceberg/public-api.txt +++ b/crates/iceberg/public-api.txt @@ -71,16 +71,8 @@ pub struct iceberg::arrow::ArrowReaderBuilder impl iceberg::arrow::ArrowReaderBuilder pub fn iceberg::arrow::ArrowReaderBuilder::build(self) -> iceberg::arrow::ArrowReader pub fn iceberg::arrow::ArrowReaderBuilder::new(file_io: iceberg::io::FileIO, runtime: iceberg::Runtime) -> Self -pub fn iceberg::arrow::ArrowReaderBuilder::with_batch_size(self, batch_size: usize) -> Self -pub fn iceberg::arrow::ArrowReaderBuilder::with_bloom_filter_enabled(self, bloom_filter_enabled: bool) -> Self -pub fn iceberg::arrow::ArrowReaderBuilder::with_data_file_concurrency_limit(self, val: usize) -> Self -pub fn iceberg::arrow::ArrowReaderBuilder::with_metadata_size_hint(self, metadata_size_hint: usize) -> Self pub fn iceberg::arrow::ArrowReaderBuilder::with_options(self, options: iceberg::arrow::ArrowReaderOptions) -> Self pub fn iceberg::arrow::ArrowReaderBuilder::with_parquet_options(self, options: iceberg::arrow::ParquetReadOptions) -> Self -pub fn iceberg::arrow::ArrowReaderBuilder::with_range_coalesce_bytes(self, range_coalesce_bytes: u64) -> Self -pub fn iceberg::arrow::ArrowReaderBuilder::with_range_fetch_concurrency(self, range_fetch_concurrency: usize) -> Self -pub fn iceberg::arrow::ArrowReaderBuilder::with_row_group_filtering_enabled(self, row_group_filtering_enabled: bool) -> Self -pub fn iceberg::arrow::ArrowReaderBuilder::with_row_selection_enabled(self, row_selection_enabled: bool) -> Self pub struct iceberg::arrow::ArrowReaderOptions impl iceberg::arrow::ArrowReaderOptions pub fn iceberg::arrow::ArrowReaderOptions::batch_size(&self) -> core::option::Option @@ -91,13 +83,10 @@ pub fn iceberg::arrow::ArrowReaderOptions::row_group_filtering_enabled(&self) -> pub fn iceberg::arrow::ArrowReaderOptions::row_selection_enabled(&self) -> bool impl core::clone::Clone for iceberg::arrow::ArrowReaderOptions pub fn iceberg::arrow::ArrowReaderOptions::clone(&self) -> iceberg::arrow::ArrowReaderOptions -impl core::cmp::PartialEq for iceberg::arrow::ArrowReaderOptions -pub fn iceberg::arrow::ArrowReaderOptions::eq(&self, other: &iceberg::arrow::ArrowReaderOptions) -> bool impl core::default::Default for iceberg::arrow::ArrowReaderOptions pub fn iceberg::arrow::ArrowReaderOptions::default() -> Self impl core::fmt::Debug for iceberg::arrow::ArrowReaderOptions pub fn iceberg::arrow::ArrowReaderOptions::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result -impl core::marker::StructuralPartialEq for iceberg::arrow::ArrowReaderOptions impl iceberg::arrow::ArrowReaderOptions pub fn iceberg::arrow::ArrowReaderOptions::builder() -> ArrowReaderOptionsBuilder<((), (), (), (), (), ())> impl serde_core::ser::Serialize for iceberg::arrow::ArrowReaderOptions @@ -106,19 +95,15 @@ impl<'de> serde_core::de::Deserialize<'de> for iceberg::arrow::ArrowReaderOption pub fn iceberg::arrow::ArrowReaderOptions::deserialize<__D>(__deserializer: __D) -> core::result::Result::Error> where __D: serde_core::de::Deserializer<'de> pub struct iceberg::arrow::ParquetReadOptions impl iceberg::arrow::ParquetReadOptions -pub fn iceberg::arrow::ParquetReadOptions::metadata_size_hint(&self) -> core::option::Option +pub fn iceberg::arrow::ParquetReadOptions::metadata_size_hint(&self) -> usize pub fn iceberg::arrow::ParquetReadOptions::range_coalesce_bytes(&self) -> u64 pub fn iceberg::arrow::ParquetReadOptions::range_fetch_concurrency(&self) -> usize impl core::clone::Clone for iceberg::arrow::ParquetReadOptions pub fn iceberg::arrow::ParquetReadOptions::clone(&self) -> iceberg::arrow::ParquetReadOptions -impl core::cmp::PartialEq for iceberg::arrow::ParquetReadOptions -pub fn iceberg::arrow::ParquetReadOptions::eq(&self, other: &iceberg::arrow::ParquetReadOptions) -> bool impl core::default::Default for iceberg::arrow::ParquetReadOptions pub fn iceberg::arrow::ParquetReadOptions::default() -> Self impl core::fmt::Debug for iceberg::arrow::ParquetReadOptions pub fn iceberg::arrow::ParquetReadOptions::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result -impl core::marker::Copy for iceberg::arrow::ParquetReadOptions -impl core::marker::StructuralPartialEq for iceberg::arrow::ParquetReadOptions impl iceberg::arrow::ParquetReadOptions pub fn iceberg::arrow::ParquetReadOptions::builder() -> ParquetReadOptionsBuilder<((), (), ())> impl serde_core::ser::Serialize for iceberg::arrow::ParquetReadOptions diff --git a/crates/iceberg/src/arrow/delete_file_loader.rs b/crates/iceberg/src/arrow/delete_file_loader.rs index ec61710a42..b37ef713f0 100644 --- a/crates/iceberg/src/arrow/delete_file_loader.rs +++ b/crates/iceberg/src/arrow/delete_file_loader.rs @@ -80,6 +80,7 @@ impl BasicDeleteFileLoader { &self.file_io, file_size_in_bytes, parquet_read_options, + false, self.scan_metrics.bytes_read_counter(), key_metadata, ) diff --git a/crates/iceberg/src/arrow/reader/file_reader.rs b/crates/iceberg/src/arrow/reader/file_reader.rs index 63b7f06758..aef9522497 100644 --- a/crates/iceberg/src/arrow/reader/file_reader.rs +++ b/crates/iceberg/src/arrow/reader/file_reader.rs @@ -34,6 +34,7 @@ use crate::io::{FileMetadata, FileRead}; pub struct ArrowFileReader { meta: FileMetadata, parquet_read_options: ParquetReadOptions, + preload_page_index: bool, r: Box, } @@ -43,10 +44,16 @@ impl ArrowFileReader { Self { meta, parquet_read_options: ParquetReadOptions::builder().build(), + preload_page_index: false, r, } } + pub(super) fn with_preload_page_index(mut self, enabled: bool) -> Self { + self.preload_page_index = enabled; + self + } + /// Configure all Parquet read options. pub(crate) fn with_parquet_read_options(mut self, options: ParquetReadOptions) -> Self { self.parquet_read_options = options; @@ -118,17 +125,11 @@ impl AsyncFileReader for ArrowFileReader { async move { let reader = ParquetMetaDataReader::new() - .with_prefetch_hint(self.parquet_read_options.metadata_size_hint()) + .with_prefetch_hint(Some(self.parquet_read_options.metadata_size_hint())) // Set the page policy first because it updates both column and offset policies. - .with_page_index_policy(PageIndexPolicy::from( - self.parquet_read_options.preload_page_index(), - )) - .with_column_index_policy(PageIndexPolicy::from( - self.parquet_read_options.preload_column_index(), - )) - .with_offset_index_policy(PageIndexPolicy::from( - self.parquet_read_options.preload_offset_index(), - )) + .with_page_index_policy(PageIndexPolicy::from(self.preload_page_index)) + .with_column_index_policy(PageIndexPolicy::from(true)) + .with_offset_index_policy(PageIndexPolicy::from(true)) .with_metadata_options(metadata_options) .with_decryption_properties(decryption_properties); let size = self.meta.size; diff --git a/crates/iceberg/src/arrow/reader/mod.rs b/crates/iceberg/src/arrow/reader/mod.rs index 59a9594dcd..900707515e 100644 --- a/crates/iceberg/src/arrow/reader/mod.rs +++ b/crates/iceberg/src/arrow/reader/mod.rs @@ -43,9 +43,7 @@ mod projection; mod row_filter; mod row_lineage; pub use file_reader::ArrowFileReader; -pub use options::{ - ArrowReaderOptions, ArrowReaderOptionsBuilder, ParquetReadOptions, ParquetReadOptionsBuilder, -}; +pub use options::{ArrowReaderOptions, ParquetReadOptions}; use predicate_visitor::{CollectFieldIdVisitor, PredicateConverter}; use projection::{ add_fallback_field_ids_to_arrow_schema, apply_name_mapping_to_arrow_schema, @@ -69,80 +67,6 @@ impl ArrowReaderBuilder { } } - /// Sets the max number of in flight data files that are being fetched - pub fn with_data_file_concurrency_limit(mut self, val: usize) -> Self { - self.options.set_data_file_concurrency_limit(Some(val)); - self - } - - /// Sets the desired size of batches in the response - /// to something other than the default - pub fn with_batch_size(mut self, batch_size: usize) -> Self { - self.options.set_batch_size(Some(batch_size)); - self - } - - /// Determines whether to enable row group filtering. - pub fn with_row_group_filtering_enabled(mut self, row_group_filtering_enabled: bool) -> Self { - self.options - .set_row_group_filtering_enabled(row_group_filtering_enabled); - self - } - - /// Determines whether to enable row selection. - pub fn with_row_selection_enabled(mut self, row_selection_enabled: bool) -> Self { - self.options - .set_row_selection_enabled(row_selection_enabled); - self - } - - /// Determines whether to enable bloom filter-based row group filtering. - /// - /// When enabled, if a read is performed with an equality or IN predicate, - /// the bloom filter for relevant columns in each row group is read and - /// checked. Row groups where the bloom filter proves the value is absent - /// are skipped entirely. - /// - /// Defaults to disabled. Each bloom filter is a separate read, and they are - /// issued serially — one round trip per relevant column per row group, before - /// any data is read. TODO(#3191) - pub fn with_bloom_filter_enabled(mut self, bloom_filter_enabled: bool) -> Self { - self.options.set_bloom_filter_enabled(bloom_filter_enabled); - self - } - - /// Provide a hint as to the number of bytes to prefetch for parsing the Parquet metadata - /// - /// This hint can help reduce the number of fetch requests. For more details see the - /// [ParquetMetaDataReader documentation](https://docs.rs/parquet/latest/parquet/file/metadata/struct.ParquetMetaDataReader.html#method.with_prefetch_hint). - pub fn with_metadata_size_hint(mut self, metadata_size_hint: usize) -> Self { - self.options - .parquet_options_mut() - .set_metadata_size_hint(Some(metadata_size_hint)); - self - } - - /// Sets the gap threshold for merging nearby byte ranges into a single request. - /// Ranges with gaps smaller than this value will be coalesced. - /// - /// Defaults to 1 MiB, matching object_store's OBJECT_STORE_COALESCE_DEFAULT. - pub fn with_range_coalesce_bytes(mut self, range_coalesce_bytes: u64) -> Self { - self.options - .parquet_options_mut() - .set_range_coalesce_bytes(range_coalesce_bytes); - self - } - - /// Sets the maximum number of merged byte ranges to fetch concurrently. - /// - /// Defaults to 10, matching object_store's OBJECT_STORE_COALESCE_PARALLEL. - pub fn with_range_fetch_concurrency(mut self, range_fetch_concurrency: usize) -> Self { - self.options - .parquet_options_mut() - .set_range_fetch_concurrency(range_fetch_concurrency); - self - } - /// Replaces all reader settings while preserving local I/O and runtime resources. pub fn with_options(mut self, options: ArrowReaderOptions) -> Self { self.options = options; @@ -160,7 +84,8 @@ impl ArrowReaderBuilder { let concurrency_limit_data_files = self .options .data_file_concurrency_limit() - .unwrap_or_else(|| available_parallelism().get()); + .unwrap_or_else(|| available_parallelism().get()) + .max(1); ArrowReader { batch_size: self.options.batch_size(), file_io: self.file_io.clone(), @@ -173,7 +98,8 @@ impl ArrowReaderBuilder { row_group_filtering_enabled: self.options.row_group_filtering_enabled(), row_selection_enabled: self.options.row_selection_enabled(), bloom_filter_enabled: self.options.bloom_filter_enabled(), - parquet_read_options: *self.options.parquet_options(), + parquet_read_options: self.options.parquet_options().clone(), + runtime: self.runtime.clone(), } } } diff --git a/crates/iceberg/src/arrow/reader/options.rs b/crates/iceberg/src/arrow/reader/options.rs index f93dc62f8b..ebc1ff113e 100644 --- a/crates/iceberg/src/arrow/reader/options.rs +++ b/crates/iceberg/src/arrow/reader/options.rs @@ -24,11 +24,16 @@ use super::{ /// Configures Arrow reads independently of file planning and local I/O resources. /// +/// Serializable alongside planned file tasks so readers can be constructed later. +/// In self-describing map formats such as JSON, field names are part of the +/// serialized format: missing fields use defaults and unknown fields are ignored. +/// Compatibility across different crate versions is not otherwise guaranteed. +/// /// ```no_run /// # use iceberg::{Result, arrow::{ArrowReaderOptions, ParquetReadOptions}, table::Table}; /// # async fn read(table: &Table) -> Result<()> { /// let options = ArrowReaderOptions::builder() -/// .with_batch_size(Some(1024)) +/// .with_batch_size(1024) /// .with_parquet_options( /// ParquetReadOptions::builder() /// .with_metadata_size_hint(512 * 1024) @@ -39,13 +44,16 @@ use super::{ /// # Ok(()) /// # } /// ``` -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, TypedBuilder)] +#[derive(Debug, Clone, Serialize, Deserialize, TypedBuilder)] +#[cfg_attr(test, derive(PartialEq))] #[serde(default)] #[builder(field_defaults(default, setter(prefix = "with_")))] pub struct ArrowReaderOptions { /// Desired batch size, or the reader default when unset. + #[builder(default = None, setter(strip_option))] batch_size: Option, /// Maximum in-flight data files, or execution-host parallelism when unset. + #[builder(default = None, setter(strip_option))] data_file_concurrency_limit: Option, /// Whether to prune row groups using their statistics. #[builder(default = true)] @@ -70,45 +78,38 @@ impl ArrowReaderOptions { self.batch_size } - pub(super) fn parquet_options_mut(&mut self) -> &mut ParquetReadOptions { - &mut self.parquet_options - } - - pub(super) fn set_batch_size(&mut self, value: Option) { - self.batch_size = value; - } /// Returns the configured data file concurrency limit. + /// + /// Unset limits use execution-host parallelism; zero is treated as one. pub fn data_file_concurrency_limit(&self) -> Option { self.data_file_concurrency_limit } - pub(super) fn set_data_file_concurrency_limit(&mut self, value: Option) { - self.data_file_concurrency_limit = value; - } /// Returns whether row-group statistics filtering is enabled. + /// + /// Enabled by default: statistics can skip groups that cannot match the + /// predicate, generally improving performance with little overhead. pub fn row_group_filtering_enabled(&self) -> bool { self.row_group_filtering_enabled } - pub(super) fn set_row_group_filtering_enabled(&mut self, value: bool) { - self.row_group_filtering_enabled = value; - } /// Returns whether page-index row selection is enabled. + /// + /// Disabled by default: parsing page indexes can cost more than the reads + /// it avoids. Evaluate it against the table's sorting, partitioning, row-group + /// size, and page size before enabling it. pub fn row_selection_enabled(&self) -> bool { self.row_selection_enabled } - pub(super) fn set_row_selection_enabled(&mut self, value: bool) { - self.row_selection_enabled = value; - } /// Returns whether bloom-filter pruning is enabled. + /// + /// Disabled by default: bloom filters require extra reads per relevant + /// column and row group. They can eliminate groups for equality and IN predicates. pub fn bloom_filter_enabled(&self) -> bool { self.bloom_filter_enabled } - pub(super) fn set_bloom_filter_enabled(&mut self, value: bool) { - self.bloom_filter_enabled = value; - } /// Returns the configured parquet options. pub fn parquet_options(&self) -> &ParquetReadOptions { &self.parquet_options @@ -120,7 +121,8 @@ impl ArrowReaderOptions { } /// Options for tuning Parquet file I/O. -#[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize, TypedBuilder)] +#[derive(Clone, Debug, Serialize, Deserialize, TypedBuilder)] +#[cfg_attr(test, derive(PartialEq))] #[serde(default)] #[builder(field_defaults(setter(prefix = "with_")))] pub struct ParquetReadOptions { @@ -130,8 +132,8 @@ pub struct ParquetReadOptions { /// [ParquetMetaDataReader documentation](https://docs.rs/parquet/latest/parquet/file/metadata/struct.ParquetMetaDataReader.html#method.with_prefetch_hint). /// /// Defaults to 512 KiB, matching DataFusion's default `ParquetOptions::metadata_size_hint`. - #[builder(default = Some(DEFAULT_METADATA_SIZE_HINT), setter(strip_option))] - metadata_size_hint: Option, + #[builder(default = DEFAULT_METADATA_SIZE_HINT)] + metadata_size_hint: usize, /// Gap threshold for merging nearby byte ranges into a single request. /// Ranges with gaps smaller than this value will be coalesced. /// @@ -143,18 +145,6 @@ pub struct ParquetReadOptions { /// Defaults to 10, matching object_store's `OBJECT_STORE_COALESCE_PARALLEL`. #[builder(default = DEFAULT_RANGE_FETCH_CONCURRENCY)] range_fetch_concurrency: usize, - /// Whether to preload the column index when reading Parquet metadata. - #[builder(default = true, setter(skip))] - #[serde(skip)] - preload_column_index: bool, - /// Whether to preload the offset index when reading Parquet metadata. - #[builder(default = true, setter(skip))] - #[serde(skip)] - preload_offset_index: bool, - /// Whether to preload the page index when reading Parquet metadata. - #[builder(default = false, setter(skip))] - #[serde(skip)] - preload_page_index: bool, } impl Default for ParquetReadOptions { @@ -164,21 +154,8 @@ impl Default for ParquetReadOptions { } impl ParquetReadOptions { - pub(super) fn set_metadata_size_hint(&mut self, value: Option) { - self.metadata_size_hint = value; - } - pub(super) fn set_range_coalesce_bytes(&mut self, value: u64) { - self.range_coalesce_bytes = value; - } - pub(super) fn set_range_fetch_concurrency(&mut self, value: usize) { - self.range_fetch_concurrency = value; - } - pub(super) fn set_preload_page_index(&mut self, value: bool) { - self.preload_page_index = value; - } - /// Returns the configured metadata size hint. - pub fn metadata_size_hint(&self) -> Option { + pub fn metadata_size_hint(&self) -> usize { self.metadata_size_hint } @@ -191,28 +168,42 @@ impl ParquetReadOptions { pub fn range_fetch_concurrency(&self) -> usize { self.range_fetch_concurrency } - - pub(super) fn preload_column_index(&self) -> bool { - self.preload_column_index - } - - pub(super) fn preload_offset_index(&self) -> bool { - self.preload_offset_index - } - - pub(super) fn preload_page_index(&self) -> bool { - self.preload_page_index - } } #[cfg(test)] mod tests { + use std::time::Duration; + + use futures::{TryStreamExt, stream}; + use super::*; use crate::arrow::ArrowReaderBuilder; use crate::io::FileIO; use crate::test_utils::test_runtime; use crate::util::available_parallelism; + #[tokio::test] + async fn deserialized_zero_concurrency_completes_reads() { + let options: ArrowReaderOptions = + serde_json::from_str(r#"{"data_file_concurrency_limit":0}"#).unwrap(); + let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) + .with_options(options) + .build(); + assert_eq!(reader.concurrency_limit_data_files, 1); + let batches = tokio::time::timeout( + Duration::from_secs(1), + reader + .read(Box::pin(stream::empty())) + .unwrap() + .stream() + .try_collect::>(), + ) + .await + .expect("zero concurrency must not stall the task stream") + .unwrap(); + assert!(batches.is_empty()); + } + #[test] fn missing_options_use_defaults_and_unknown_options_are_ignored() { let decoded: ArrowReaderOptions = @@ -221,9 +212,7 @@ mod tests { let decoded: ArrowReaderOptions = serde_json::from_str(r#"{"batch_size":17}"#).unwrap(); assert_eq!( decoded, - ArrowReaderOptions::builder() - .with_batch_size(Some(17)) - .build() + ArrowReaderOptions::builder().with_batch_size(17).build() ); assert_eq!( serde_json::to_value(ArrowReaderOptions::default()).unwrap()["data_file_concurrency_limit"], @@ -250,30 +239,18 @@ mod tests { } #[test] - fn parquet_override_preserves_reader_settings_and_allows_individual_overrides() { + fn parquet_override_preserves_other_reader_settings() { + let parquet = ParquetReadOptions::builder() + .with_metadata_size_hint(4096) + .with_range_fetch_concurrency(2) + .build(); let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) - .with_options( - ArrowReaderOptions::builder() - .with_batch_size(Some(17)) - .build(), - ) - .with_parquet_options( - ParquetReadOptions::builder() - .with_metadata_size_hint(4096) - .with_range_fetch_concurrency(2) - .build(), - ) - .with_range_fetch_concurrency(3) + .with_options(ArrowReaderOptions::builder().with_batch_size(17).build()) + .with_parquet_options(parquet.clone()) .build(); assert_eq!( (reader.batch_size, reader.parquet_read_options), - ( - Some(17), - ParquetReadOptions::builder() - .with_metadata_size_hint(4096) - .with_range_fetch_concurrency(3) - .build() - ) + (Some(17), parquet) ); } @@ -282,8 +259,8 @@ mod tests { for explicit in [false, true] { let options = if explicit { ArrowReaderOptions::builder() - .with_batch_size(Some(17)) - .with_data_file_concurrency_limit(Some(3)) + .with_batch_size(17) + .with_data_file_concurrency_limit(3) .with_row_group_filtering_enabled(false) .with_row_selection_enabled(true) .with_bloom_filter_enabled(true) @@ -299,11 +276,16 @@ mod tests { ArrowReaderOptions::default() }; let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) - .with_batch_size(1024) - .with_data_file_concurrency_limit(99) - .with_metadata_size_hint(4096) - .with_range_coalesce_bytes(8192) - .with_range_fetch_concurrency(2) + .with_options( + ArrowReaderOptions::builder() + .with_batch_size(1024) + .with_parquet_options( + ParquetReadOptions::builder() + .with_range_fetch_concurrency(99) + .build(), + ) + .build(), + ) .with_options(options) .build(); let expected = if explicit { @@ -328,9 +310,9 @@ mod tests { reader.parquet_read_options.range_fetch_concurrency() ), if explicit { - (Some(4096), 8192, 2) + (4096, 8192, 2) } else { - (Some(512 * 1024), 1024 * 1024, 10) + (512 * 1024, 1024 * 1024, 10) } ); } diff --git a/crates/iceberg/src/arrow/reader/pipeline.rs b/crates/iceberg/src/arrow/reader/pipeline.rs index 2206e3289d..05fafa75e7 100644 --- a/crates/iceberg/src/arrow/reader/pipeline.rs +++ b/crates/iceberg/src/arrow/reader/pipeline.rs @@ -153,8 +153,6 @@ impl FileScanTaskReader { async fn process(self, task: FileScanTask) -> Result { let should_load_page_index = (self.row_selection_enabled && task.predicate().is_some()) || !task.deletes().is_empty(); - let mut parquet_read_options = self.parquet_read_options; - parquet_read_options.set_preload_page_index(should_load_page_index); let delete_filter_rx = self .delete_file_loader @@ -169,6 +167,7 @@ impl FileScanTaskReader { let file_size_in_bytes = task.file_size_in_bytes(); let bytes_read = Arc::clone(self.scan_metrics.bytes_read_counter()); let key_metadata = task.key_metadata().map(|km| km.to_vec()); + let parquet_read_options = self.parquet_read_options.clone(); let opening = self .runtime .io() @@ -178,6 +177,7 @@ impl FileScanTaskReader { &file_io, file_size_in_bytes, parquet_read_options, + should_load_page_index, &bytes_read, key_metadata.as_deref(), ) @@ -817,6 +817,7 @@ impl ArrowReader { file_io: &FileIO, file_size_in_bytes: u64, parquet_read_options: ParquetReadOptions, + preload_page_index: bool, bytes_read: &Arc, key_metadata: Option<&[u8]>, ) -> Result<(ArrowFileReader, ArrowReaderMetadata)> { @@ -827,6 +828,7 @@ impl ArrowReader { Box::new(counting_reader), file_size_in_bytes, parquet_read_options, + preload_page_index, key_metadata, ) .await @@ -836,6 +838,7 @@ impl ArrowReader { parquet_reader: Box, file_size_in_bytes: u64, parquet_read_options: ParquetReadOptions, + preload_page_index: bool, key_metadata: Option<&[u8]>, ) -> Result<(ArrowFileReader, ArrowReaderMetadata)> { let mut reader = ArrowFileReader::new( @@ -844,7 +847,8 @@ impl ArrowReader { }, parquet_reader, ) - .with_parquet_read_options(parquet_read_options); + .with_parquet_read_options(parquet_read_options) + .with_preload_page_index(preload_page_index); let arrow_reader_options = Self::build_arrow_reader_options(key_metadata)?; @@ -903,8 +907,8 @@ mod tests { use tempfile::TempDir; use crate::Runtime; - use crate::arrow::ArrowReaderBuilder; use crate::arrow::test_utils::write_encrypted_parquet; + use crate::arrow::{ArrowReaderBuilder, ArrowReaderOptions as ReaderOptions}; use crate::io::FileIO; use crate::metadata_columns::{ RESERVED_COL_NAME_POS, RESERVED_COL_NAME_ROW_ID, RESERVED_FIELD_ID_FILE, @@ -2330,7 +2334,11 @@ mod tests { // Row selection must be enabled for the predicate to produce a RowSelection. let reader = ArrowReaderBuilder::new(FileIO::new_with_fs(), Runtime::current()) - .with_row_selection_enabled(true) + .with_options( + ReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) .build(); let tasks = Box::pin(futures::stream::iter(vec![Ok(task)])) as FileScanTaskStream; let batches: Vec = reader @@ -2371,7 +2379,11 @@ mod tests { ); let reader = ArrowReaderBuilder::new(FileIO::new_with_fs(), Runtime::current()) - .with_row_selection_enabled(true) + .with_options( + ReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) .build(); let tasks = Box::pin(futures::stream::iter(vec![Ok(task)])) as FileScanTaskStream; let batches: Vec = reader @@ -2722,7 +2734,11 @@ mod tests { // Read with concurrency=1 (fast-path) let reader = ArrowReaderBuilder::new(file_io, Runtime::current()) - .with_data_file_concurrency_limit(1) + .with_options( + ReaderOptions::builder() + .with_data_file_concurrency_limit(1) + .build(), + ) .build(); // Create tasks in a specific order: file_0, file_1, file_2 @@ -3554,7 +3570,11 @@ mod tests { // reads `id` for its own evaluation even though `id` is not projected; the surviving // rows must keep their ABSOLUTE positions (0 and 2), not renumbered (0 and 1). let reader = ArrowReaderBuilder::new(FileIO::new_with_fs(), Runtime::current()) - .with_row_selection_enabled(true) + .with_options( + ReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) .build(); let tasks = Box::pin(futures::stream::iter(vec![Ok(task)])) as FileScanTaskStream; let batches: Vec = reader diff --git a/crates/iceberg/src/arrow/reader/projection.rs b/crates/iceberg/src/arrow/reader/projection.rs index 3474b21624..054489ec6b 100644 --- a/crates/iceberg/src/arrow/reader/projection.rs +++ b/crates/iceberg/src/arrow/reader/projection.rs @@ -588,7 +588,7 @@ mod tests { use parquet::schema::types::SchemaDescriptor; use tempfile::TempDir; - use crate::arrow::{ArrowReader, ArrowReaderBuilder}; + use crate::arrow::{ArrowReader, ArrowReaderBuilder, ArrowReaderOptions as ReaderOptions}; use crate::expr::{Bind, Reference}; use crate::io::FileIO; use crate::scan::{FileScanTask, FileScanTaskStream}; @@ -1127,8 +1127,12 @@ message schema { let predicate = Reference::new("name").equal_to(Datum::string("Alice")); let reader = ArrowReaderBuilder::new(file_io, Runtime::current()) - .with_row_group_filtering_enabled(true) - .with_row_selection_enabled(true) + .with_options( + ReaderOptions::builder() + .with_row_group_filtering_enabled(true) + .with_row_selection_enabled(true) + .build(), + ) .build(); let tasks = Box::pin(futures::stream::iter( @@ -1965,8 +1969,12 @@ message schema { // Enable both row_group_filtering and row_selection - triggered the panic let reader = ArrowReaderBuilder::new(file_io, Runtime::current()) - .with_row_group_filtering_enabled(true) - .with_row_selection_enabled(true) + .with_options( + ReaderOptions::builder() + .with_row_group_filtering_enabled(true) + .with_row_selection_enabled(true) + .build(), + ) .build(); let tasks = Box::pin(futures::stream::iter( @@ -2320,8 +2328,12 @@ message schema { let predicate = Reference::new("id").greater_than(Datum::int(1)); let reader = ArrowReaderBuilder::new(FileIO::new_with_fs(), Runtime::current()) - .with_row_group_filtering_enabled(true) - .with_row_selection_enabled(true) + .with_options( + ReaderOptions::builder() + .with_row_group_filtering_enabled(true) + .with_row_selection_enabled(true) + .build(), + ) .build(); let tasks = Box::pin(futures::stream::iter( diff --git a/crates/iceberg/src/arrow/reader/row_filter.rs b/crates/iceberg/src/arrow/reader/row_filter.rs index 331e7f9656..7e9746adbc 100644 --- a/crates/iceberg/src/arrow/reader/row_filter.rs +++ b/crates/iceberg/src/arrow/reader/row_filter.rs @@ -242,7 +242,9 @@ mod tests { use tempfile::TempDir; use crate::Runtime; - use crate::arrow::{ArrowReader, ArrowReaderBuilder}; + use crate::arrow::{ + ArrowReader, ArrowReaderBuilder, ArrowReaderOptions as ReaderOptions, ParquetReadOptions, + }; use crate::expr::{Bind, BoundPredicate, Predicate, Reference}; use crate::io::FileIO; use crate::scan::{FileScanTask, FileScanTaskDeleteFile, FileScanTaskStream}; @@ -686,7 +688,11 @@ mod tests { .unwrap(); let reader = ArrowReaderBuilder::new(FileIO::new_with_fs(), Runtime::current()) - .with_row_group_filtering_enabled(true) + .with_options( + ReaderOptions::builder() + .with_row_group_filtering_enabled(true) + .build(), + ) .build(); let tasks = Box::pin(futures::stream::iter(vec![Ok(task)])) as FileScanTaskStream; let result = reader @@ -1013,7 +1019,11 @@ mod tests { ); let reader = ArrowReaderBuilder::new(FileIO::new_with_fs(), Runtime::current()) - .with_row_group_filtering_enabled(true) + .with_options( + ReaderOptions::builder() + .with_row_group_filtering_enabled(true) + .build(), + ) .build(); let task = FileScanTask::builder() @@ -1093,8 +1103,12 @@ mod tests { assert!(matches!(predicate, BoundPredicate::Not(_))); let reader = ArrowReaderBuilder::new(FileIO::new_with_fs(), Runtime::current()) - .with_row_group_filtering_enabled(false) - .with_row_selection_enabled(true) + .with_options( + ReaderOptions::builder() + .with_row_group_filtering_enabled(false) + .with_row_selection_enabled(true) + .build(), + ) .build(); let task = FileScanTask::builder() @@ -1380,7 +1394,11 @@ mod tests { let file_io = FileIO::new_with_fs(); let reader = ArrowReaderBuilder::new(file_io.clone(), Runtime::current()) // Enabling row selection () - .with_row_selection_enabled(true) + .with_options( + ReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) .build(); let file_size = std::fs::metadata(&file_path).unwrap().len(); @@ -1468,7 +1486,11 @@ mod tests { .unwrap(); let reader_sub2 = ArrowReaderBuilder::new(file_io.clone(), Runtime::current()) - .with_row_selection_enabled(true) + .with_options( + ReaderOptions::builder() + .with_row_selection_enabled(true) + .build(), + ) .build(); let task_sub2 = FileScanTask::builder() @@ -1582,10 +1604,18 @@ mod tests { ) -> (Vec, u64) { let file_io = FileIO::new_with_fs(); let reader = ArrowReaderBuilder::new(file_io, Runtime::current()) - .with_bloom_filter_enabled(bloom_enabled) + .with_options( + ReaderOptions::builder() + .with_bloom_filter_enabled(bloom_enabled) + .build(), + ) // Keep the fixed footer prefetch small so the byte measurement reflects // row group I/O rather than a constant metadata read. - .with_metadata_size_hint(8 * 1024) + .with_parquet_options( + ParquetReadOptions::builder() + .with_metadata_size_hint(8 * 1024) + .build(), + ) .build(); let task = FileScanTask::builder() diff --git a/crates/iceberg/src/cow_rewrite/mod.rs b/crates/iceberg/src/cow_rewrite/mod.rs index 25d5fb4b8f..fa567834bd 100644 --- a/crates/iceberg/src/cow_rewrite/mod.rs +++ b/crates/iceberg/src/cow_rewrite/mod.rs @@ -80,6 +80,7 @@ use futures::TryStreamExt; pub use plan::CowRewriteFile; pub use rewriter::{CowBatchRewrite, CowBatchRewriter}; +use crate::arrow::ArrowReaderOptions; use crate::expr::Predicate; use crate::scan::FileScanTaskStream; use crate::spec::{DataFile, PartitionKey}; @@ -273,7 +274,11 @@ impl<'a> CowRewriteBuilder<'a> { // candidates are fetched once per file. let mut reader_builder = self.table.reader_builder(); if let Some(batch_size) = self.batch_size { - reader_builder = reader_builder.with_batch_size(batch_size); + reader_builder = reader_builder.with_options( + ArrowReaderOptions::builder() + .with_batch_size(batch_size) + .build(), + ); } let mut batches = reader_builder.build().read(tasks)?.stream(); diff --git a/crates/iceberg/src/scan/mod.rs b/crates/iceberg/src/scan/mod.rs index 5f57e5f128..06405542fb 100644 --- a/crates/iceberg/src/scan/mod.rs +++ b/crates/iceberg/src/scan/mod.rs @@ -1899,8 +1899,8 @@ mod tests { .build() .unwrap(); let options = ArrowReaderOptions::builder() - .with_batch_size(Some(17)) - .with_data_file_concurrency_limit(Some(1)) + .with_batch_size(17) + .with_data_file_concurrency_limit(1) .with_parquet_options( ParquetReadOptions::builder() .with_metadata_size_hint(4096) @@ -1908,6 +1908,7 @@ mod tests { .build(), ) .with_row_selection_enabled(true) + .with_bloom_filter_enabled(true) .build(); let expected: Vec<_> = scan .to_arrow(options.clone()) @@ -1929,11 +1930,13 @@ mod tests { serde_json::json!({ "batch_size": 17, "data_file_concurrency_limit": 1, "row_group_filtering_enabled": true, "row_selection_enabled": true, - "bloom_filter_enabled": false, + "bloom_filter_enabled": true, "parquet_options": { "metadata_size_hint": 4096, "range_coalesce_bytes": 1048576, "range_fetch_concurrency": 2 }, }) ); - let options: ArrowReaderOptions = serde_json::from_value(serialized).unwrap(); + let deserialized: ArrowReaderOptions = serde_json::from_value(serialized).unwrap(); + assert_eq!(deserialized, options); + let options = deserialized; drop(scan); let reader = ArrowReaderBuilder::new(FileIO::new_with_fs(), Runtime::try_current().unwrap()) From 0f438d02aecf6ce12b3890f03526fd65d1bc748b Mon Sep 17 00:00:00 2001 From: "alex.bianchi" Date: Fri, 30 Oct 2026 14:10:17 -0400 Subject: [PATCH 06/15] fix: preserve scan concurrency defaults for Arrow reads --- crates/iceberg/README.md | 8 ++-- crates/iceberg/src/arrow/reader/options.rs | 31 +++++++++++++- crates/iceberg/src/scan/mod.rs | 47 +++++++++++++++++++++- 3 files changed, 79 insertions(+), 7 deletions(-) diff --git a/crates/iceberg/README.md b/crates/iceberg/README.md index 84edca3dbc..fb15084dd8 100644 --- a/crates/iceberg/README.md +++ b/crates/iceberg/README.md @@ -85,9 +85,11 @@ let stream = table.scan().build()?.to_arrow(options).await?; When upgrading, move reader-setting calls from `TableScanBuilder` to `ArrowReaderOptions::builder()` and pass the result to `to_arrow(options)`. Use `to_arrow(Default::default())` for default settings. Scan -`with_concurrency_limit()` controls manifest planning only; set -`with_data_file_concurrency_limit(limit)` on the reader options to also -limit data-file reads. An unset data-file limit uses execution-host parallelism. +`with_concurrency_limit()` controls manifest planning and supplies the default +data-file limit for `to_arrow()`. Set `with_data_file_concurrency_limit(limit)` +on the reader options to override that limit independently. For direct reader +construction, an unset data-file limit uses execution-host parallelism. Zero +reader concurrency is treated as one. Locally constructed readers accept the same options through `ArrowReaderBuilder::with_options(options)`, including Parquet I/O tuning. diff --git a/crates/iceberg/src/arrow/reader/options.rs b/crates/iceberg/src/arrow/reader/options.rs index ebc1ff113e..5e8de47a2f 100644 --- a/crates/iceberg/src/arrow/reader/options.rs +++ b/crates/iceberg/src/arrow/reader/options.rs @@ -52,7 +52,7 @@ pub struct ArrowReaderOptions { /// Desired batch size, or the reader default when unset. #[builder(default = None, setter(strip_option))] batch_size: Option, - /// Maximum in-flight data files, or execution-host parallelism when unset. + /// Maximum in-flight data files, or the construction path's default when unset. #[builder(default = None, setter(strip_option))] data_file_concurrency_limit: Option, /// Whether to prune row groups using their statistics. @@ -73,6 +73,11 @@ impl Default for ArrowReaderOptions { } impl ArrowReaderOptions { + pub(crate) fn with_default_data_file_concurrency_limit(mut self, limit: usize) -> Self { + self.data_file_concurrency_limit.get_or_insert(limit); + self + } + /// Returns the configured batch size. pub fn batch_size(&self) -> Option { self.batch_size @@ -80,7 +85,8 @@ impl ArrowReaderOptions { /// Returns the configured data file concurrency limit. /// - /// Unset limits use execution-host parallelism; zero is treated as one. + /// When unset, `TableScan::to_arrow` inherits the scan's data-file limit, while + /// direct reader construction uses execution-host parallelism. Zero is treated as one. pub fn data_file_concurrency_limit(&self) -> Option { self.data_file_concurrency_limit } @@ -204,6 +210,27 @@ mod tests { assert!(batches.is_empty()); } + #[test] + fn default_data_file_concurrency_preserves_explicit_options() { + for (configured, expected) in [(None, 7), (Some(3), 3), (Some(0), 0)] { + let options: ArrowReaderOptions = serde_json::from_value(serde_json::json!({ + "batch_size": 17, + "data_file_concurrency_limit": configured, + "row_selection_enabled": true, + })) + .unwrap(); + let expected = ArrowReaderOptions::builder() + .with_batch_size(17) + .with_data_file_concurrency_limit(expected) + .with_row_selection_enabled(true) + .build(); + assert_eq!( + options.with_default_data_file_concurrency_limit(7), + expected + ); + } + } + #[test] fn missing_options_use_defaults_and_unknown_options_are_ignored() { let decoded: ArrowReaderOptions = diff --git a/crates/iceberg/src/scan/mod.rs b/crates/iceberg/src/scan/mod.rs index 06405542fb..8a0d5720bf 100644 --- a/crates/iceberg/src/scan/mod.rs +++ b/crates/iceberg/src/scan/mod.rs @@ -147,6 +147,7 @@ pub struct TableScanBuilder<'a> { filter: Option, concurrency_limit_manifest_entries: usize, concurrency_limit_manifest_files: usize, + concurrency_limit_data_files: usize, } impl<'a> TableScanBuilder<'a> { @@ -161,6 +162,7 @@ impl<'a> TableScanBuilder<'a> { filter: None, concurrency_limit_manifest_entries: num_cpus, concurrency_limit_manifest_files: num_cpus, + concurrency_limit_data_files: num_cpus, } } @@ -237,11 +239,11 @@ impl<'a> TableScanBuilder<'a> { self } - /// Sets the concurrency limit for both manifest files and manifest - /// entries for this scan + /// Sets manifest concurrency and the default data-file concurrency for `to_arrow`. pub fn with_concurrency_limit(mut self, limit: usize) -> Self { self.concurrency_limit_manifest_files = limit; self.concurrency_limit_manifest_entries = limit; + self.concurrency_limit_data_files = limit; self } @@ -285,6 +287,7 @@ impl<'a> TableScanBuilder<'a> { plan_context: None, concurrency_limit_manifest_entries: self.concurrency_limit_manifest_entries, concurrency_limit_manifest_files: self.concurrency_limit_manifest_files, + concurrency_limit_data_files: self.concurrency_limit_data_files, runtime: self.table.runtime().clone(), }); }; @@ -344,6 +347,7 @@ impl<'a> TableScanBuilder<'a> { plan_context: Some(plan_context), concurrency_limit_manifest_entries: self.concurrency_limit_manifest_entries, concurrency_limit_manifest_files: self.concurrency_limit_manifest_files, + concurrency_limit_data_files: self.concurrency_limit_data_files, runtime: self.table.runtime().clone(), }) } @@ -361,6 +365,7 @@ pub struct TableScan { /// The maximum number of manifest files that will be /// retrieved from [`FileIO`] concurrently concurrency_limit_manifest_files: usize, + concurrency_limit_data_files: usize, /// The maximum number of [`ManifestEntry`]s that will /// be processed in parallel @@ -512,7 +517,13 @@ impl TableScan { } /// Returns an [`ArrowRecordBatchStream`]. + /// + /// An unset reader concurrency inherits the scan's data-file limit; an explicit + /// reader limit overrides it. Zero reader concurrency is treated as one. pub async fn to_arrow(&self, options: ArrowReaderOptions) -> Result { + // Preserve the scan's data-file concurrency bound for backwards compatibility. + let options = + options.with_default_data_file_concurrency_limit(self.concurrency_limit_data_files); ArrowReaderBuilder::new(self.file_io.clone(), self.runtime.clone()) .with_options(options) .build() @@ -671,6 +682,7 @@ mod tests { ROWS_PER_FIXTURE_FILE, TableTestFixture, assert_last_updated_seq_all, }; use crate::test_utils::test_runtime; + use crate::util::available_parallelism; use crate::util::snapshot::snapshot_id_as_of_time; use crate::{ErrorKind, Runtime, TableIdent}; @@ -3193,6 +3205,37 @@ mod tests { assert!(err.to_string().contains("not found")); } + #[test] + fn test_scan_retains_data_file_concurrency() { + for fixture in [TableTestFixture::new(), TableTestFixture::new_empty()] { + let default_scan = fixture.table.scan().build().unwrap(); + let default_limit = available_parallelism().get(); + assert_eq!( + ( + default_scan.concurrency_limit_manifest_files, + default_scan.concurrency_limit_manifest_entries, + default_scan.concurrency_limit_data_files + ), + (default_limit, default_limit, default_limit), + ); + let scan = fixture + .table + .scan() + .with_concurrency_limit(3) + .with_manifest_entry_concurrency_limit(1) + .build() + .unwrap(); + assert_eq!( + ( + scan.concurrency_limit_manifest_files, + scan.concurrency_limit_manifest_entries, + scan.concurrency_limit_data_files + ), + (3, 1, 3), + ); + } + } + #[tokio::test] async fn test_scan_deadlock() { let mut fixture = TableTestFixture::new(); From a9a695ab1211cc4cc80946c218895e044880dd0d Mon Sep 17 00:00:00 2001 From: "alex.bianchi" Date: Mon, 2 Nov 2026 12:09:19 -0500 Subject: [PATCH 07/15] refactor(arrow): clarify options API and focus reader tests --- Cargo.lock | 1 + crates/iceberg/Cargo.toml | 1 + crates/iceberg/public-api.txt | 6 + crates/iceberg/src/arrow/reader/mod.rs | 2 + crates/iceberg/src/arrow/reader/options.rs | 181 +++++---------------- crates/iceberg/src/arrow/reader/tests.rs | 117 +++++++++++++ 6 files changed, 168 insertions(+), 140 deletions(-) create mode 100644 crates/iceberg/src/arrow/reader/tests.rs diff --git a/Cargo.lock b/Cargo.lock index 1feac7435c..37b0e10199 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2910,6 +2910,7 @@ dependencies = [ "regex", "reqwest 0.12.28", "roaring", + "rstest", "serde", "serde_arrow", "serde_bytes", diff --git a/crates/iceberg/Cargo.toml b/crates/iceberg/Cargo.toml index fa5e6cce73..a6f1bb7d35 100644 --- a/crates/iceberg/Cargo.toml +++ b/crates/iceberg/Cargo.toml @@ -99,6 +99,7 @@ mockall = { workspace = true } pretty_assertions = { workspace = true } rand = { workspace = true } regex = { workspace = true } +rstest = { workspace = true } serde_arrow = { version = "0.14", features = ["arrow-59"] } tempfile = { workspace = true } tracing-subscriber = { workspace = true } diff --git a/crates/iceberg/public-api.txt b/crates/iceberg/public-api.txt index 1a6e23e2b3..cfbfbb0c5b 100644 --- a/crates/iceberg/public-api.txt +++ b/crates/iceberg/public-api.txt @@ -83,10 +83,13 @@ pub fn iceberg::arrow::ArrowReaderOptions::row_group_filtering_enabled(&self) -> pub fn iceberg::arrow::ArrowReaderOptions::row_selection_enabled(&self) -> bool impl core::clone::Clone for iceberg::arrow::ArrowReaderOptions pub fn iceberg::arrow::ArrowReaderOptions::clone(&self) -> iceberg::arrow::ArrowReaderOptions +impl core::cmp::PartialEq for iceberg::arrow::ArrowReaderOptions +pub fn iceberg::arrow::ArrowReaderOptions::eq(&self, other: &iceberg::arrow::ArrowReaderOptions) -> bool impl core::default::Default for iceberg::arrow::ArrowReaderOptions pub fn iceberg::arrow::ArrowReaderOptions::default() -> Self impl core::fmt::Debug for iceberg::arrow::ArrowReaderOptions pub fn iceberg::arrow::ArrowReaderOptions::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for iceberg::arrow::ArrowReaderOptions impl iceberg::arrow::ArrowReaderOptions pub fn iceberg::arrow::ArrowReaderOptions::builder() -> ArrowReaderOptionsBuilder<((), (), (), (), (), ())> impl serde_core::ser::Serialize for iceberg::arrow::ArrowReaderOptions @@ -100,10 +103,13 @@ pub fn iceberg::arrow::ParquetReadOptions::range_coalesce_bytes(&self) -> u64 pub fn iceberg::arrow::ParquetReadOptions::range_fetch_concurrency(&self) -> usize impl core::clone::Clone for iceberg::arrow::ParquetReadOptions pub fn iceberg::arrow::ParquetReadOptions::clone(&self) -> iceberg::arrow::ParquetReadOptions +impl core::cmp::PartialEq for iceberg::arrow::ParquetReadOptions +pub fn iceberg::arrow::ParquetReadOptions::eq(&self, other: &iceberg::arrow::ParquetReadOptions) -> bool impl core::default::Default for iceberg::arrow::ParquetReadOptions pub fn iceberg::arrow::ParquetReadOptions::default() -> Self impl core::fmt::Debug for iceberg::arrow::ParquetReadOptions pub fn iceberg::arrow::ParquetReadOptions::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for iceberg::arrow::ParquetReadOptions impl iceberg::arrow::ParquetReadOptions pub fn iceberg::arrow::ParquetReadOptions::builder() -> ParquetReadOptionsBuilder<((), (), ())> impl serde_core::ser::Serialize for iceberg::arrow::ParquetReadOptions diff --git a/crates/iceberg/src/arrow/reader/mod.rs b/crates/iceberg/src/arrow/reader/mod.rs index 900707515e..52e119300c 100644 --- a/crates/iceberg/src/arrow/reader/mod.rs +++ b/crates/iceberg/src/arrow/reader/mod.rs @@ -42,6 +42,8 @@ mod predicate_visitor; mod projection; mod row_filter; mod row_lineage; +#[cfg(test)] +mod tests; pub use file_reader::ArrowFileReader; pub use options::{ArrowReaderOptions, ParquetReadOptions}; use predicate_visitor::{CollectFieldIdVisitor, PredicateConverter}; diff --git a/crates/iceberg/src/arrow/reader/options.rs b/crates/iceberg/src/arrow/reader/options.rs index 5e8de47a2f..8b0c1a5989 100644 --- a/crates/iceberg/src/arrow/reader/options.rs +++ b/crates/iceberg/src/arrow/reader/options.rs @@ -44,8 +44,7 @@ use super::{ /// # Ok(()) /// # } /// ``` -#[derive(Debug, Clone, Serialize, Deserialize, TypedBuilder)] -#[cfg_attr(test, derive(PartialEq))] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, TypedBuilder)] #[serde(default)] #[builder(field_defaults(default, setter(prefix = "with_")))] pub struct ArrowReaderOptions { @@ -73,6 +72,7 @@ impl Default for ArrowReaderOptions { } impl ArrowReaderOptions { + /// Supplies the scan's fallback limit without replacing an explicit reader limit. pub(crate) fn with_default_data_file_concurrency_limit(mut self, limit: usize) -> Self { self.data_file_concurrency_limit.get_or_insert(limit); self @@ -127,8 +127,7 @@ impl ArrowReaderOptions { } /// Options for tuning Parquet file I/O. -#[derive(Clone, Debug, Serialize, Deserialize, TypedBuilder)] -#[cfg_attr(test, derive(PartialEq))] +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, TypedBuilder)] #[serde(default)] #[builder(field_defaults(setter(prefix = "with_")))] pub struct ParquetReadOptions { @@ -178,170 +177,72 @@ impl ParquetReadOptions { #[cfg(test)] mod tests { - use std::time::Duration; - - use futures::{TryStreamExt, stream}; + use rstest::rstest; use super::*; - use crate::arrow::ArrowReaderBuilder; - use crate::io::FileIO; - use crate::test_utils::test_runtime; - use crate::util::available_parallelism; - #[tokio::test] - async fn deserialized_zero_concurrency_completes_reads() { - let options: ArrowReaderOptions = - serde_json::from_str(r#"{"data_file_concurrency_limit":0}"#).unwrap(); - let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) - .with_options(options) - .build(); - assert_eq!(reader.concurrency_limit_data_files, 1); - let batches = tokio::time::timeout( - Duration::from_secs(1), - reader - .read(Box::pin(stream::empty())) - .unwrap() - .stream() - .try_collect::>(), - ) - .await - .expect("zero concurrency must not stall the task stream") + #[rstest] + #[case::unset(None, 7)] + #[case::explicit(Some(3), 3)] + #[case::zero(Some(0), 0)] + fn default_data_file_concurrency_preserves_explicit_options( + #[case] configured: Option, + #[case] expected: usize, + ) { + // Include unrelated settings to check that resolving concurrency preserves them. + let options: ArrowReaderOptions = serde_json::from_value(serde_json::json!({ + "batch_size": 17, + "data_file_concurrency_limit": configured, + "row_selection_enabled": true, + })) .unwrap(); - assert!(batches.is_empty()); + // Resolve the scan fallback; explicit zero is normalized later by the reader. + let options = options.with_default_data_file_concurrency_limit(7); + let expected = ArrowReaderOptions::builder() + .with_batch_size(17) + .with_data_file_concurrency_limit(expected) + .with_row_selection_enabled(true) + .build(); + assert_eq!(options, expected); } #[test] - fn default_data_file_concurrency_preserves_explicit_options() { - for (configured, expected) in [(None, 7), (Some(3), 3), (Some(0), 0)] { - let options: ArrowReaderOptions = serde_json::from_value(serde_json::json!({ - "batch_size": 17, - "data_file_concurrency_limit": configured, - "row_selection_enabled": true, - })) - .unwrap(); - let expected = ArrowReaderOptions::builder() - .with_batch_size(17) - .with_data_file_concurrency_limit(expected) - .with_row_selection_enabled(true) - .build(); - assert_eq!( - options.with_default_data_file_concurrency_limit(7), - expected - ); - } + fn missing_options_use_defaults() { + let decoded: ArrowReaderOptions = serde_json::from_str(r#"{"batch_size":17}"#).unwrap(); + assert_eq!( + decoded, + ArrowReaderOptions::builder().with_batch_size(17).build() + ); } #[test] - fn missing_options_use_defaults_and_unknown_options_are_ignored() { + fn unknown_options_are_ignored() { let decoded: ArrowReaderOptions = serde_json::from_str(r#"{"future_option":true}"#).unwrap(); assert_eq!(decoded, ArrowReaderOptions::default()); - let decoded: ArrowReaderOptions = serde_json::from_str(r#"{"batch_size":17}"#).unwrap(); - assert_eq!( - decoded, - ArrowReaderOptions::builder().with_batch_size(17).build() - ); + } + + #[test] + fn unset_concurrency_serializes_as_null() { assert_eq!( serde_json::to_value(ArrowReaderOptions::default()).unwrap()["data_file_concurrency_limit"], - serde_json::Value::Null + serde_json::Value::Null, ); } #[test] fn partial_parquet_options_preserve_defaults() { - let options: ArrowReaderOptions = serde_json::from_str( - r#"{"parquet_options":{"range_fetch_concurrency":2,"future_option":true}}"#, - ) - .unwrap(); + let options: ArrowReaderOptions = + serde_json::from_str(r#"{"parquet_options":{"range_fetch_concurrency":2}}"#).unwrap(); assert_eq!( options, ArrowReaderOptions::builder() .with_parquet_options( ParquetReadOptions::builder() .with_range_fetch_concurrency(2) - .build() + .build(), ) .build() ); } - - #[test] - fn parquet_override_preserves_other_reader_settings() { - let parquet = ParquetReadOptions::builder() - .with_metadata_size_hint(4096) - .with_range_fetch_concurrency(2) - .build(); - let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) - .with_options(ArrowReaderOptions::builder().with_batch_size(17).build()) - .with_parquet_options(parquet.clone()) - .build(); - assert_eq!( - (reader.batch_size, reader.parquet_read_options), - (Some(17), parquet) - ); - } - - #[test] - fn options_restore_local_defaults_or_apply_explicit_settings() { - for explicit in [false, true] { - let options = if explicit { - ArrowReaderOptions::builder() - .with_batch_size(17) - .with_data_file_concurrency_limit(3) - .with_row_group_filtering_enabled(false) - .with_row_selection_enabled(true) - .with_bloom_filter_enabled(true) - .with_parquet_options( - ParquetReadOptions::builder() - .with_metadata_size_hint(4096) - .with_range_coalesce_bytes(8192) - .with_range_fetch_concurrency(2) - .build(), - ) - .build() - } else { - ArrowReaderOptions::default() - }; - let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) - .with_options( - ArrowReaderOptions::builder() - .with_batch_size(1024) - .with_parquet_options( - ParquetReadOptions::builder() - .with_range_fetch_concurrency(99) - .build(), - ) - .build(), - ) - .with_options(options) - .build(); - let expected = if explicit { - (Some(17), 3, false, true, true) - } else { - (None, available_parallelism().get(), true, false, false) - }; - assert_eq!( - ( - reader.batch_size, - reader.concurrency_limit_data_files, - reader.row_group_filtering_enabled, - reader.row_selection_enabled, - reader.bloom_filter_enabled - ), - expected - ); - assert_eq!( - ( - reader.parquet_read_options.metadata_size_hint(), - reader.parquet_read_options.range_coalesce_bytes(), - reader.parquet_read_options.range_fetch_concurrency() - ), - if explicit { - (4096, 8192, 2) - } else { - (512 * 1024, 1024 * 1024, 10) - } - ); - } - } } diff --git a/crates/iceberg/src/arrow/reader/tests.rs b/crates/iceberg/src/arrow/reader/tests.rs new file mode 100644 index 0000000000..82becc0633 --- /dev/null +++ b/crates/iceberg/src/arrow/reader/tests.rs @@ -0,0 +1,117 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use std::time::Duration; + +use futures::{TryStreamExt, stream}; + +use super::{ArrowReaderBuilder, ArrowReaderOptions, ParquetReadOptions}; +use crate::io::FileIO; +use crate::test_utils::test_runtime; +use crate::util::available_parallelism; + +#[tokio::test] +async fn deserialized_zero_concurrency_completes_reads() { + let options: ArrowReaderOptions = + serde_json::from_str(r#"{"data_file_concurrency_limit":0}"#).unwrap(); + let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) + .with_options(options) + .build(); + assert_eq!(reader.concurrency_limit_data_files, 1); + let batches = tokio::time::timeout( + Duration::from_secs(1), + reader + .read(Box::pin(stream::empty())) + .unwrap() + .stream() + .try_collect::>(), + ) + .await + .expect("zero concurrency must not stall the task stream") + .unwrap(); + assert!(batches.is_empty()); +} + +#[test] +fn default_options_restore_local_reader_defaults() { + let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) + .with_options( + ArrowReaderOptions::builder() + .with_batch_size(1024) + .with_data_file_concurrency_limit(3) + .with_row_group_filtering_enabled(false) + .with_row_selection_enabled(true) + .with_bloom_filter_enabled(true) + .with_parquet_options( + ParquetReadOptions::builder() + .with_range_fetch_concurrency(99) + .build(), + ) + .build(), + ) + .with_options(ArrowReaderOptions::default()) + .build(); + assert_eq!( + ( + reader.batch_size, + reader.concurrency_limit_data_files, + reader.row_group_filtering_enabled, + reader.row_selection_enabled, + reader.bloom_filter_enabled, + reader.parquet_read_options + ), + ( + None, + available_parallelism().get(), + true, + false, + false, + ParquetReadOptions::default() + ), + ); +} + +#[test] +fn explicit_options_configure_reader_settings() { + let parquet = ParquetReadOptions::builder() + .with_metadata_size_hint(4096) + .with_range_coalesce_bytes(8192) + .with_range_fetch_concurrency(2) + .build(); + let options = ArrowReaderOptions::builder() + .with_batch_size(17) + .with_data_file_concurrency_limit(3) + .with_row_group_filtering_enabled(false) + .with_row_selection_enabled(true) + .with_bloom_filter_enabled(true) + .with_parquet_options(parquet.clone()) + .build(); + let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) + .with_options(options) + .build(); + assert_eq!( + ( + reader.batch_size, + reader.concurrency_limit_data_files, + reader.row_group_filtering_enabled, + reader.row_selection_enabled, + reader.bloom_filter_enabled, + reader.parquet_read_options + ), + (Some(17), 3, false, true, true, parquet), + ); +} From 6afe7b2b555466fa85e2e22655ff60e174aeb248 Mon Sep 17 00:00:00 2001 From: "alex.bianchi" Date: Tue, 3 Nov 2026 09:15:58 -0500 Subject: [PATCH 08/15] test(arrow): migrate upstream scheduling tests to reader options --- crates/iceberg/src/arrow/reader/pipeline.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/iceberg/src/arrow/reader/pipeline.rs b/crates/iceberg/src/arrow/reader/pipeline.rs index 05fafa75e7..a4c183638e 100644 --- a/crates/iceberg/src/arrow/reader/pipeline.rs +++ b/crates/iceberg/src/arrow/reader/pipeline.rs @@ -3977,6 +3977,7 @@ mod tests { use super::*; use crate::Result; + use crate::arrow::ParquetReadOptions; use crate::arrow::scan_metrics::ScanMetrics; use crate::expr::{Bind, Predicate, Reference}; use crate::io::{ @@ -4161,9 +4162,17 @@ mod tests { let file_io = FileIOBuilder::new(Arc::new(DeferredStorageFactory { storage })).build(); let reader = ArrowReaderBuilder::new(file_io, Runtime::current()) - .with_data_file_concurrency_limit(CONCURRENCY) - .with_metadata_size_hint(METADATA_SIZE_HINT) - .with_bloom_filter_enabled(bound.is_some()) + .with_options( + ReaderOptions::builder() + .with_data_file_concurrency_limit(CONCURRENCY) + .with_bloom_filter_enabled(bound.is_some()) + .with_parquet_options( + ParquetReadOptions::builder() + .with_metadata_size_hint(METADATA_SIZE_HINT) + .build(), + ) + .build(), + ) .build(); let tasks = Box::pin(futures::stream::iter(tasks.into_iter().map(Ok))) as FileScanTaskStream; From 59841bf945dd2b9c505ca47f781be8b64a14fed6 Mon Sep 17 00:00:00 2001 From: "alex.bianchi" Date: Wed, 4 Nov 2026 09:33:39 -0500 Subject: [PATCH 09/15] refactor(arrow): retain reader options and address review feedback --- .../iceberg/src/arrow/delete_file_loader.rs | 1 - .../iceberg/src/arrow/reader/file_reader.rs | 19 ++- crates/iceberg/src/arrow/reader/mod.rs | 100 ++++++++++++--- crates/iceberg/src/arrow/reader/options.rs | 50 +++++++- crates/iceberg/src/arrow/reader/pipeline.rs | 22 ++-- crates/iceberg/src/arrow/reader/tests.rs | 117 ------------------ crates/iceberg/src/scan/mod.rs | 2 +- 7 files changed, 153 insertions(+), 158 deletions(-) delete mode 100644 crates/iceberg/src/arrow/reader/tests.rs diff --git a/crates/iceberg/src/arrow/delete_file_loader.rs b/crates/iceberg/src/arrow/delete_file_loader.rs index b37ef713f0..ec61710a42 100644 --- a/crates/iceberg/src/arrow/delete_file_loader.rs +++ b/crates/iceberg/src/arrow/delete_file_loader.rs @@ -80,7 +80,6 @@ impl BasicDeleteFileLoader { &self.file_io, file_size_in_bytes, parquet_read_options, - false, self.scan_metrics.bytes_read_counter(), key_metadata, ) diff --git a/crates/iceberg/src/arrow/reader/file_reader.rs b/crates/iceberg/src/arrow/reader/file_reader.rs index aef9522497..22355737a1 100644 --- a/crates/iceberg/src/arrow/reader/file_reader.rs +++ b/crates/iceberg/src/arrow/reader/file_reader.rs @@ -34,7 +34,6 @@ use crate::io::{FileMetadata, FileRead}; pub struct ArrowFileReader { meta: FileMetadata, parquet_read_options: ParquetReadOptions, - preload_page_index: bool, r: Box, } @@ -44,16 +43,10 @@ impl ArrowFileReader { Self { meta, parquet_read_options: ParquetReadOptions::builder().build(), - preload_page_index: false, r, } } - pub(super) fn with_preload_page_index(mut self, enabled: bool) -> Self { - self.preload_page_index = enabled; - self - } - /// Configure all Parquet read options. pub(crate) fn with_parquet_read_options(mut self, options: ParquetReadOptions) -> Self { self.parquet_read_options = options; @@ -127,9 +120,15 @@ impl AsyncFileReader for ArrowFileReader { let reader = ParquetMetaDataReader::new() .with_prefetch_hint(Some(self.parquet_read_options.metadata_size_hint())) // Set the page policy first because it updates both column and offset policies. - .with_page_index_policy(PageIndexPolicy::from(self.preload_page_index)) - .with_column_index_policy(PageIndexPolicy::from(true)) - .with_offset_index_policy(PageIndexPolicy::from(true)) + .with_page_index_policy(PageIndexPolicy::from( + self.parquet_read_options.preload_page_index(), + )) + .with_column_index_policy(PageIndexPolicy::from( + self.parquet_read_options.preload_column_index(), + )) + .with_offset_index_policy(PageIndexPolicy::from( + self.parquet_read_options.preload_offset_index(), + )) .with_metadata_options(metadata_options) .with_decryption_properties(decryption_properties); let size = self.meta.size; diff --git a/crates/iceberg/src/arrow/reader/mod.rs b/crates/iceberg/src/arrow/reader/mod.rs index 52e119300c..fc2d464af9 100644 --- a/crates/iceberg/src/arrow/reader/mod.rs +++ b/crates/iceberg/src/arrow/reader/mod.rs @@ -42,8 +42,6 @@ mod predicate_visitor; mod projection; mod row_filter; mod row_lineage; -#[cfg(test)] -mod tests; pub use file_reader::ArrowFileReader; pub use options::{ArrowReaderOptions, ParquetReadOptions}; use predicate_visitor::{CollectFieldIdVisitor, PredicateConverter}; @@ -77,7 +75,7 @@ impl ArrowReaderBuilder { /// Replaces Parquet I/O settings while preserving other reader settings. pub fn with_parquet_options(mut self, options: ParquetReadOptions) -> Self { - self.options.set_parquet_options(options); + self.options = self.options.with_parquet_options(options); self } @@ -89,7 +87,7 @@ impl ArrowReaderBuilder { .unwrap_or_else(|| available_parallelism().get()) .max(1); ArrowReader { - batch_size: self.options.batch_size(), + options: self.options, file_io: self.file_io.clone(), delete_file_loader: CachingDeleteFileLoader::new( self.file_io.clone(), @@ -97,10 +95,6 @@ impl ArrowReaderBuilder { self.runtime.clone(), ), concurrency_limit_data_files, - row_group_filtering_enabled: self.options.row_group_filtering_enabled(), - row_selection_enabled: self.options.row_selection_enabled(), - bloom_filter_enabled: self.options.bloom_filter_enabled(), - parquet_read_options: self.options.parquet_options().clone(), runtime: self.runtime.clone(), } } @@ -109,16 +103,94 @@ impl ArrowReaderBuilder { /// Reads data from Parquet files #[derive(Clone)] pub struct ArrowReader { - batch_size: Option, + options: ArrowReaderOptions, file_io: FileIO, delete_file_loader: CachingDeleteFileLoader, - /// the maximum number of data files that can be fetched at the same time + /// Effective concurrency after resolving the host default and normalizing zero. concurrency_limit_data_files: usize, - row_group_filtering_enabled: bool, - row_selection_enabled: bool, - bloom_filter_enabled: bool, - parquet_read_options: ParquetReadOptions, runtime: Runtime, } + +#[cfg(test)] +mod tests { + use std::time::Duration; + + use futures::{TryStreamExt, stream}; + + use super::{ArrowReaderBuilder, ArrowReaderOptions, ParquetReadOptions}; + use crate::io::FileIO; + use crate::test_utils::test_runtime; + use crate::util::available_parallelism; + + #[tokio::test] + async fn deserialized_zero_concurrency_completes_reads() { + let options: ArrowReaderOptions = + serde_json::from_str(r#"{"data_file_concurrency_limit":0}"#).unwrap(); + let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) + .with_options(options) + .build(); + assert_eq!(reader.concurrency_limit_data_files, 1); + let batches = tokio::time::timeout( + Duration::from_secs(1), + reader + .read(Box::pin(stream::empty())) + .unwrap() + .stream() + .try_collect::>(), + ) + .await + .expect("zero concurrency must not stall the task stream") + .unwrap(); + assert!(batches.is_empty()); + } + + #[test] + fn default_options_restore_local_reader_defaults() { + let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) + .with_options( + ArrowReaderOptions::builder() + .with_batch_size(1024) + .with_data_file_concurrency_limit(3) + .with_row_group_filtering_enabled(false) + .with_row_selection_enabled(true) + .with_bloom_filter_enabled(true) + .with_parquet_options( + ParquetReadOptions::builder() + .with_range_fetch_concurrency(99) + .build(), + ) + .build(), + ) + .with_options(ArrowReaderOptions::default()) + .build(); + assert_eq!(reader.options, ArrowReaderOptions::default()); + assert_eq!( + reader.concurrency_limit_data_files, + available_parallelism().get() + ); + } + + #[test] + fn explicit_options_configure_reader_settings() { + let parquet = ParquetReadOptions::builder() + .with_metadata_size_hint(4096) + .with_range_coalesce_bytes(8192) + .with_range_fetch_concurrency(2) + .build(); + let options = ArrowReaderOptions::builder() + .with_batch_size(17) + .with_data_file_concurrency_limit(3) + .with_row_group_filtering_enabled(false) + .with_row_selection_enabled(true) + .with_bloom_filter_enabled(true) + .with_parquet_options(parquet) + .build(); + let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) + .with_options(options.clone()) + .build(); + assert_eq!(reader.options, options); + assert_eq!(reader.concurrency_limit_data_files, 3); + } +} diff --git a/crates/iceberg/src/arrow/reader/options.rs b/crates/iceberg/src/arrow/reader/options.rs index 8b0c1a5989..a58ed09cd6 100644 --- a/crates/iceberg/src/arrow/reader/options.rs +++ b/crates/iceberg/src/arrow/reader/options.rs @@ -73,7 +73,7 @@ impl Default for ArrowReaderOptions { impl ArrowReaderOptions { /// Supplies the scan's fallback limit without replacing an explicit reader limit. - pub(crate) fn with_default_data_file_concurrency_limit(mut self, limit: usize) -> Self { + pub(crate) fn with_data_file_concurrency_limit_if_unset(mut self, limit: usize) -> Self { self.data_file_concurrency_limit.get_or_insert(limit); self } @@ -121,8 +121,9 @@ impl ArrowReaderOptions { &self.parquet_options } - pub(super) fn set_parquet_options(&mut self, value: ParquetReadOptions) { + pub(super) fn with_parquet_options(mut self, value: ParquetReadOptions) -> Self { self.parquet_options = value; + self } } @@ -150,6 +151,16 @@ pub struct ParquetReadOptions { /// Defaults to 10, matching object_store's `OBJECT_STORE_COALESCE_PARALLEL`. #[builder(default = DEFAULT_RANGE_FETCH_CONCURRENCY)] range_fetch_concurrency: usize, + // Index preloading is derived during execution, not serialized reader configuration. + #[serde(skip)] + #[builder(default = true, setter(skip))] + preload_column_index: bool, + #[serde(skip)] + #[builder(default = true, setter(skip))] + preload_offset_index: bool, + #[serde(skip)] + #[builder(default = false, setter(skip))] + preload_page_index: bool, } impl Default for ParquetReadOptions { @@ -159,6 +170,23 @@ impl Default for ParquetReadOptions { } impl ParquetReadOptions { + pub(super) fn with_preload_page_index(mut self, enabled: bool) -> Self { + self.preload_page_index = enabled; + self + } + + pub(super) fn preload_column_index(&self) -> bool { + self.preload_column_index + } + + pub(super) fn preload_offset_index(&self) -> bool { + self.preload_offset_index + } + + pub(super) fn preload_page_index(&self) -> bool { + self.preload_page_index + } + /// Returns the configured metadata size hint. pub fn metadata_size_hint(&self) -> usize { self.metadata_size_hint @@ -197,7 +225,7 @@ mod tests { })) .unwrap(); // Resolve the scan fallback; explicit zero is normalized later by the reader. - let options = options.with_default_data_file_concurrency_limit(7); + let options = options.with_data_file_concurrency_limit_if_unset(7); let expected = ArrowReaderOptions::builder() .with_batch_size(17) .with_data_file_concurrency_limit(expected) @@ -230,6 +258,22 @@ mod tests { ); } + #[test] + fn parquet_serialization_excludes_execution_settings() { + let options = ParquetReadOptions::default().with_preload_page_index(true); + let encoded = serde_json::to_value(options).unwrap(); + assert_eq!( + encoded, + serde_json::json!({ + "metadata_size_hint": DEFAULT_METADATA_SIZE_HINT, + "range_coalesce_bytes": DEFAULT_RANGE_COALESCE_BYTES, + "range_fetch_concurrency": DEFAULT_RANGE_FETCH_CONCURRENCY, + }), + ); + let decoded: ParquetReadOptions = serde_json::from_value(encoded).unwrap(); + assert_eq!(decoded, ParquetReadOptions::default()); + } + #[test] fn partial_parquet_options_preserve_defaults() { let options: ArrowReaderOptions = diff --git a/crates/iceberg/src/arrow/reader/pipeline.rs b/crates/iceberg/src/arrow/reader/pipeline.rs index a4c183638e..f011927de1 100644 --- a/crates/iceberg/src/arrow/reader/pipeline.rs +++ b/crates/iceberg/src/arrow/reader/pipeline.rs @@ -69,15 +69,15 @@ impl ArrowReader { let scan_metrics = ScanMetrics::new(); let task_reader = FileScanTaskReader { - batch_size: self.batch_size, + batch_size: self.options.batch_size(), file_io: self.file_io, delete_file_loader: self .delete_file_loader .with_scan_metrics(scan_metrics.clone()), - row_group_filtering_enabled: self.row_group_filtering_enabled, - row_selection_enabled: self.row_selection_enabled, - bloom_filter_enabled: self.bloom_filter_enabled, - parquet_read_options: self.parquet_read_options, + row_group_filtering_enabled: self.options.row_group_filtering_enabled(), + row_selection_enabled: self.options.row_selection_enabled(), + bloom_filter_enabled: self.options.bloom_filter_enabled(), + parquet_read_options: self.options.parquet_options().clone(), scan_metrics: scan_metrics.clone(), runtime: self.runtime, }; @@ -167,7 +167,10 @@ impl FileScanTaskReader { let file_size_in_bytes = task.file_size_in_bytes(); let bytes_read = Arc::clone(self.scan_metrics.bytes_read_counter()); let key_metadata = task.key_metadata().map(|km| km.to_vec()); - let parquet_read_options = self.parquet_read_options.clone(); + let parquet_read_options = self + .parquet_read_options + .clone() + .with_preload_page_index(should_load_page_index); let opening = self .runtime .io() @@ -177,7 +180,6 @@ impl FileScanTaskReader { &file_io, file_size_in_bytes, parquet_read_options, - should_load_page_index, &bytes_read, key_metadata.as_deref(), ) @@ -817,7 +819,6 @@ impl ArrowReader { file_io: &FileIO, file_size_in_bytes: u64, parquet_read_options: ParquetReadOptions, - preload_page_index: bool, bytes_read: &Arc, key_metadata: Option<&[u8]>, ) -> Result<(ArrowFileReader, ArrowReaderMetadata)> { @@ -828,7 +829,6 @@ impl ArrowReader { Box::new(counting_reader), file_size_in_bytes, parquet_read_options, - preload_page_index, key_metadata, ) .await @@ -838,7 +838,6 @@ impl ArrowReader { parquet_reader: Box, file_size_in_bytes: u64, parquet_read_options: ParquetReadOptions, - preload_page_index: bool, key_metadata: Option<&[u8]>, ) -> Result<(ArrowFileReader, ArrowReaderMetadata)> { let mut reader = ArrowFileReader::new( @@ -847,8 +846,7 @@ impl ArrowReader { }, parquet_reader, ) - .with_parquet_read_options(parquet_read_options) - .with_preload_page_index(preload_page_index); + .with_parquet_read_options(parquet_read_options); let arrow_reader_options = Self::build_arrow_reader_options(key_metadata)?; diff --git a/crates/iceberg/src/arrow/reader/tests.rs b/crates/iceberg/src/arrow/reader/tests.rs deleted file mode 100644 index 82becc0633..0000000000 --- a/crates/iceberg/src/arrow/reader/tests.rs +++ /dev/null @@ -1,117 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -use std::time::Duration; - -use futures::{TryStreamExt, stream}; - -use super::{ArrowReaderBuilder, ArrowReaderOptions, ParquetReadOptions}; -use crate::io::FileIO; -use crate::test_utils::test_runtime; -use crate::util::available_parallelism; - -#[tokio::test] -async fn deserialized_zero_concurrency_completes_reads() { - let options: ArrowReaderOptions = - serde_json::from_str(r#"{"data_file_concurrency_limit":0}"#).unwrap(); - let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) - .with_options(options) - .build(); - assert_eq!(reader.concurrency_limit_data_files, 1); - let batches = tokio::time::timeout( - Duration::from_secs(1), - reader - .read(Box::pin(stream::empty())) - .unwrap() - .stream() - .try_collect::>(), - ) - .await - .expect("zero concurrency must not stall the task stream") - .unwrap(); - assert!(batches.is_empty()); -} - -#[test] -fn default_options_restore_local_reader_defaults() { - let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) - .with_options( - ArrowReaderOptions::builder() - .with_batch_size(1024) - .with_data_file_concurrency_limit(3) - .with_row_group_filtering_enabled(false) - .with_row_selection_enabled(true) - .with_bloom_filter_enabled(true) - .with_parquet_options( - ParquetReadOptions::builder() - .with_range_fetch_concurrency(99) - .build(), - ) - .build(), - ) - .with_options(ArrowReaderOptions::default()) - .build(); - assert_eq!( - ( - reader.batch_size, - reader.concurrency_limit_data_files, - reader.row_group_filtering_enabled, - reader.row_selection_enabled, - reader.bloom_filter_enabled, - reader.parquet_read_options - ), - ( - None, - available_parallelism().get(), - true, - false, - false, - ParquetReadOptions::default() - ), - ); -} - -#[test] -fn explicit_options_configure_reader_settings() { - let parquet = ParquetReadOptions::builder() - .with_metadata_size_hint(4096) - .with_range_coalesce_bytes(8192) - .with_range_fetch_concurrency(2) - .build(); - let options = ArrowReaderOptions::builder() - .with_batch_size(17) - .with_data_file_concurrency_limit(3) - .with_row_group_filtering_enabled(false) - .with_row_selection_enabled(true) - .with_bloom_filter_enabled(true) - .with_parquet_options(parquet.clone()) - .build(); - let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) - .with_options(options) - .build(); - assert_eq!( - ( - reader.batch_size, - reader.concurrency_limit_data_files, - reader.row_group_filtering_enabled, - reader.row_selection_enabled, - reader.bloom_filter_enabled, - reader.parquet_read_options - ), - (Some(17), 3, false, true, true, parquet), - ); -} diff --git a/crates/iceberg/src/scan/mod.rs b/crates/iceberg/src/scan/mod.rs index 8a0d5720bf..79e212b73d 100644 --- a/crates/iceberg/src/scan/mod.rs +++ b/crates/iceberg/src/scan/mod.rs @@ -523,7 +523,7 @@ impl TableScan { pub async fn to_arrow(&self, options: ArrowReaderOptions) -> Result { // Preserve the scan's data-file concurrency bound for backwards compatibility. let options = - options.with_default_data_file_concurrency_limit(self.concurrency_limit_data_files); + options.with_data_file_concurrency_limit_if_unset(self.concurrency_limit_data_files); ArrowReaderBuilder::new(self.file_io.clone(), self.runtime.clone()) .with_options(options) .build() From c98a57231309d087799991562a9bac9b90df21aa Mon Sep 17 00:00:00 2001 From: "alex.bianchi" Date: Wed, 4 Nov 2026 11:31:18 -0500 Subject: [PATCH 10/15] fix(arrow): serialize Parquet preload settings --- crates/iceberg/src/arrow/reader/options.rs | 21 +++++++++++++-------- crates/iceberg/src/scan/mod.rs | 5 ++++- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/crates/iceberg/src/arrow/reader/options.rs b/crates/iceberg/src/arrow/reader/options.rs index a58ed09cd6..c8b29c6d69 100644 --- a/crates/iceberg/src/arrow/reader/options.rs +++ b/crates/iceberg/src/arrow/reader/options.rs @@ -151,14 +151,11 @@ pub struct ParquetReadOptions { /// Defaults to 10, matching object_store's `OBJECT_STORE_COALESCE_PARALLEL`. #[builder(default = DEFAULT_RANGE_FETCH_CONCURRENCY)] range_fetch_concurrency: usize, - // Index preloading is derived during execution, not serialized reader configuration. - #[serde(skip)] #[builder(default = true, setter(skip))] preload_column_index: bool, - #[serde(skip)] #[builder(default = true, setter(skip))] preload_offset_index: bool, - #[serde(skip)] + // Execution derives page-index preloading per file from predicates and deletes. #[builder(default = false, setter(skip))] preload_page_index: bool, } @@ -259,19 +256,27 @@ mod tests { } #[test] - fn parquet_serialization_excludes_execution_settings() { - let options = ParquetReadOptions::default().with_preload_page_index(true); - let encoded = serde_json::to_value(options).unwrap(); + fn parquet_serialization_preserves_preload_settings() { + let options = ParquetReadOptions { + preload_column_index: false, + preload_offset_index: false, + preload_page_index: true, + ..ParquetReadOptions::default() + }; + let encoded = serde_json::to_value(&options).unwrap(); assert_eq!( encoded, serde_json::json!({ "metadata_size_hint": DEFAULT_METADATA_SIZE_HINT, "range_coalesce_bytes": DEFAULT_RANGE_COALESCE_BYTES, "range_fetch_concurrency": DEFAULT_RANGE_FETCH_CONCURRENCY, + "preload_column_index": false, + "preload_offset_index": false, + "preload_page_index": true, }), ); let decoded: ParquetReadOptions = serde_json::from_value(encoded).unwrap(); - assert_eq!(decoded, ParquetReadOptions::default()); + assert_eq!(decoded, options); } #[test] diff --git a/crates/iceberg/src/scan/mod.rs b/crates/iceberg/src/scan/mod.rs index 79e212b73d..952c993cd8 100644 --- a/crates/iceberg/src/scan/mod.rs +++ b/crates/iceberg/src/scan/mod.rs @@ -1943,7 +1943,10 @@ mod tests { "batch_size": 17, "data_file_concurrency_limit": 1, "row_group_filtering_enabled": true, "row_selection_enabled": true, "bloom_filter_enabled": true, - "parquet_options": { "metadata_size_hint": 4096, "range_coalesce_bytes": 1048576, "range_fetch_concurrency": 2 }, + "parquet_options": { + "metadata_size_hint": 4096, "range_coalesce_bytes": 1048576, "range_fetch_concurrency": 2, + "preload_column_index": true, "preload_offset_index": true, "preload_page_index": false, + }, }) ); let deserialized: ArrowReaderOptions = serde_json::from_value(serialized).unwrap(); From 856b6518bf9a6760abd8d76f3fe73c1b1b9b1b10 Mon Sep 17 00:00:00 2001 From: "alex.bianchi" Date: Thu, 5 Nov 2026 11:01:15 -0500 Subject: [PATCH 11/15] fix(arrow): expose preload settings on Parquet options builder --- crates/iceberg/public-api.txt | 2 +- crates/iceberg/src/arrow/reader/options.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/iceberg/public-api.txt b/crates/iceberg/public-api.txt index cfbfbb0c5b..50ee19733f 100644 --- a/crates/iceberg/public-api.txt +++ b/crates/iceberg/public-api.txt @@ -111,7 +111,7 @@ impl core::fmt::Debug for iceberg::arrow::ParquetReadOptions pub fn iceberg::arrow::ParquetReadOptions::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::StructuralPartialEq for iceberg::arrow::ParquetReadOptions impl iceberg::arrow::ParquetReadOptions -pub fn iceberg::arrow::ParquetReadOptions::builder() -> ParquetReadOptionsBuilder<((), (), ())> +pub fn iceberg::arrow::ParquetReadOptions::builder() -> ParquetReadOptionsBuilder<((), (), (), (), (), ())> impl serde_core::ser::Serialize for iceberg::arrow::ParquetReadOptions pub fn iceberg::arrow::ParquetReadOptions::serialize<__S>(&self, __serializer: __S) -> core::result::Result<<__S as serde_core::ser::Serializer>::Ok, <__S as serde_core::ser::Serializer>::Error> where __S: serde_core::ser::Serializer impl<'de> serde_core::de::Deserialize<'de> for iceberg::arrow::ParquetReadOptions where iceberg::arrow::ParquetReadOptions: core::default::Default diff --git a/crates/iceberg/src/arrow/reader/options.rs b/crates/iceberg/src/arrow/reader/options.rs index c8b29c6d69..fc7e225f03 100644 --- a/crates/iceberg/src/arrow/reader/options.rs +++ b/crates/iceberg/src/arrow/reader/options.rs @@ -151,12 +151,12 @@ pub struct ParquetReadOptions { /// Defaults to 10, matching object_store's `OBJECT_STORE_COALESCE_PARALLEL`. #[builder(default = DEFAULT_RANGE_FETCH_CONCURRENCY)] range_fetch_concurrency: usize, - #[builder(default = true, setter(skip))] + #[builder(default = true)] preload_column_index: bool, - #[builder(default = true, setter(skip))] + #[builder(default = true)] preload_offset_index: bool, // Execution derives page-index preloading per file from predicates and deletes. - #[builder(default = false, setter(skip))] + #[builder(default = false)] preload_page_index: bool, } From 55452557bd4222cf24fbec08a64ec97ea15a6a9b Mon Sep 17 00:00:00 2001 From: "alex.bianchi" Date: Fri, 6 Nov 2026 13:24:36 -0500 Subject: [PATCH 12/15] fix(arrow): expose Parquet preload getters --- crates/iceberg/public-api.txt | 3 +++ crates/iceberg/src/arrow/reader/options.rs | 9 ++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/iceberg/public-api.txt b/crates/iceberg/public-api.txt index 50ee19733f..d879e2c8a1 100644 --- a/crates/iceberg/public-api.txt +++ b/crates/iceberg/public-api.txt @@ -99,6 +99,9 @@ pub fn iceberg::arrow::ArrowReaderOptions::deserialize<__D>(__deserializer: __D) pub struct iceberg::arrow::ParquetReadOptions impl iceberg::arrow::ParquetReadOptions pub fn iceberg::arrow::ParquetReadOptions::metadata_size_hint(&self) -> usize +pub fn iceberg::arrow::ParquetReadOptions::preload_column_index(&self) -> bool +pub fn iceberg::arrow::ParquetReadOptions::preload_offset_index(&self) -> bool +pub fn iceberg::arrow::ParquetReadOptions::preload_page_index(&self) -> bool pub fn iceberg::arrow::ParquetReadOptions::range_coalesce_bytes(&self) -> u64 pub fn iceberg::arrow::ParquetReadOptions::range_fetch_concurrency(&self) -> usize impl core::clone::Clone for iceberg::arrow::ParquetReadOptions diff --git a/crates/iceberg/src/arrow/reader/options.rs b/crates/iceberg/src/arrow/reader/options.rs index fc7e225f03..89cb222932 100644 --- a/crates/iceberg/src/arrow/reader/options.rs +++ b/crates/iceberg/src/arrow/reader/options.rs @@ -172,15 +172,18 @@ impl ParquetReadOptions { self } - pub(super) fn preload_column_index(&self) -> bool { + /// Returns whether to preload the column index when reading Parquet metadata. + pub fn preload_column_index(&self) -> bool { self.preload_column_index } - pub(super) fn preload_offset_index(&self) -> bool { + /// Returns whether to preload the offset index when reading Parquet metadata. + pub fn preload_offset_index(&self) -> bool { self.preload_offset_index } - pub(super) fn preload_page_index(&self) -> bool { + /// Returns whether to preload the page index when reading Parquet metadata. + pub fn preload_page_index(&self) -> bool { self.preload_page_index } From abd6687bb967ec63f7cec1168f43654b7a71e12a Mon Sep 17 00:00:00 2001 From: "alex.bianchi" Date: Mon, 9 Nov 2026 10:12:53 -0500 Subject: [PATCH 13/15] refactor(arrow): resolve reader concurrency independently of scans --- crates/iceberg/README.md | 33 --------- crates/iceberg/src/arrow/reader/mod.rs | 15 ++-- crates/iceberg/src/arrow/reader/options.rs | 10 +-- crates/iceberg/src/scan/mod.rs | 80 ++++++++++------------ 4 files changed, 51 insertions(+), 87 deletions(-) diff --git a/crates/iceberg/README.md b/crates/iceberg/README.md index fb15084dd8..bb22750519 100644 --- a/crates/iceberg/README.md +++ b/crates/iceberg/README.md @@ -65,39 +65,6 @@ async fn main() -> Result<()> { } ``` -### Reader settings - -Pass reader settings separately from file planning: - -```rust -let options = iceberg::arrow::ArrowReaderOptions::builder() - .with_batch_size(1024) - .with_row_selection_enabled(true) - .with_parquet_options( - iceberg::arrow::ParquetReadOptions::builder() - .with_metadata_size_hint(512 * 1024) - .build(), - ) - .build(); -let stream = table.scan().build()?.to_arrow(options).await?; -``` - -When upgrading, move reader-setting calls from `TableScanBuilder` to -`ArrowReaderOptions::builder()` and pass the result to `to_arrow(options)`. -Use `to_arrow(Default::default())` for default settings. Scan -`with_concurrency_limit()` controls manifest planning and supplies the default -data-file limit for `to_arrow()`. Set `with_data_file_concurrency_limit(limit)` -on the reader options to override that limit independently. For direct reader -construction, an unset data-file limit uses execution-host parallelism. Zero -reader concurrency is treated as one. - -Locally constructed readers accept the same options through -`ArrowReaderBuilder::with_options(options)`, including Parquet I/O tuning. -This replaces the complete configuration. Configure individual settings through -the options builders; per-field setters on `ArrowReaderBuilder` have been removed. -`with_parquet_options()` replaces only the nested Parquet -configuration. - ## Storage Backends For storage backend support (S3, GCS, local filesystem, etc.), use the [`iceberg-storage-opendal`](https://crates.io/crates/iceberg-storage-opendal) crate. See its [README](../storage/opendal/README.md) for available backends and feature flags. diff --git a/crates/iceberg/src/arrow/reader/mod.rs b/crates/iceberg/src/arrow/reader/mod.rs index fc2d464af9..b2f66b61c8 100644 --- a/crates/iceberg/src/arrow/reader/mod.rs +++ b/crates/iceberg/src/arrow/reader/mod.rs @@ -81,13 +81,15 @@ impl ArrowReaderBuilder { /// Build the ArrowReader. pub fn build(self) -> ArrowReader { - let concurrency_limit_data_files = self + let options = self .options + .with_data_file_concurrency_limit_if_unset(available_parallelism().get()); + let concurrency_limit_data_files = options .data_file_concurrency_limit() - .unwrap_or_else(|| available_parallelism().get()) + .expect("reader concurrency was set above") .max(1); ArrowReader { - options: self.options, + options, file_io: self.file_io.clone(), delete_file_loader: CachingDeleteFileLoader::new( self.file_io.clone(), @@ -165,7 +167,12 @@ mod tests { ) .with_options(ArrowReaderOptions::default()) .build(); - assert_eq!(reader.options, ArrowReaderOptions::default()); + assert_eq!( + reader.options, + ArrowReaderOptions::builder() + .with_data_file_concurrency_limit(available_parallelism().get()) + .build() + ); assert_eq!( reader.concurrency_limit_data_files, available_parallelism().get() diff --git a/crates/iceberg/src/arrow/reader/options.rs b/crates/iceberg/src/arrow/reader/options.rs index 89cb222932..ce69c15491 100644 --- a/crates/iceberg/src/arrow/reader/options.rs +++ b/crates/iceberg/src/arrow/reader/options.rs @@ -72,8 +72,8 @@ impl Default for ArrowReaderOptions { } impl ArrowReaderOptions { - /// Supplies the scan's fallback limit without replacing an explicit reader limit. - pub(crate) fn with_data_file_concurrency_limit_if_unset(mut self, limit: usize) -> Self { + /// Supplies a fallback limit without replacing an explicit reader limit. + pub(super) fn with_data_file_concurrency_limit_if_unset(mut self, limit: usize) -> Self { self.data_file_concurrency_limit.get_or_insert(limit); self } @@ -85,8 +85,8 @@ impl ArrowReaderOptions { /// Returns the configured data file concurrency limit. /// - /// When unset, `TableScan::to_arrow` inherits the scan's data-file limit, while - /// direct reader construction uses execution-host parallelism. Zero is treated as one. + /// When unset, reader construction uses execution-host parallelism. + /// Zero is treated as one. pub fn data_file_concurrency_limit(&self) -> Option { self.data_file_concurrency_limit } @@ -224,7 +224,7 @@ mod tests { "row_selection_enabled": true, })) .unwrap(); - // Resolve the scan fallback; explicit zero is normalized later by the reader. + // Resolve the reader fallback; explicit zero is normalized later by the reader. let options = options.with_data_file_concurrency_limit_if_unset(7); let expected = ArrowReaderOptions::builder() .with_batch_size(17) diff --git a/crates/iceberg/src/scan/mod.rs b/crates/iceberg/src/scan/mod.rs index 952c993cd8..bc103f11c9 100644 --- a/crates/iceberg/src/scan/mod.rs +++ b/crates/iceberg/src/scan/mod.rs @@ -147,7 +147,6 @@ pub struct TableScanBuilder<'a> { filter: Option, concurrency_limit_manifest_entries: usize, concurrency_limit_manifest_files: usize, - concurrency_limit_data_files: usize, } impl<'a> TableScanBuilder<'a> { @@ -162,7 +161,6 @@ impl<'a> TableScanBuilder<'a> { filter: None, concurrency_limit_manifest_entries: num_cpus, concurrency_limit_manifest_files: num_cpus, - concurrency_limit_data_files: num_cpus, } } @@ -239,11 +237,10 @@ impl<'a> TableScanBuilder<'a> { self } - /// Sets manifest concurrency and the default data-file concurrency for `to_arrow`. + /// Sets manifest file and manifest entry concurrency for file planning. pub fn with_concurrency_limit(mut self, limit: usize) -> Self { self.concurrency_limit_manifest_files = limit; self.concurrency_limit_manifest_entries = limit; - self.concurrency_limit_data_files = limit; self } @@ -287,7 +284,6 @@ impl<'a> TableScanBuilder<'a> { plan_context: None, concurrency_limit_manifest_entries: self.concurrency_limit_manifest_entries, concurrency_limit_manifest_files: self.concurrency_limit_manifest_files, - concurrency_limit_data_files: self.concurrency_limit_data_files, runtime: self.table.runtime().clone(), }); }; @@ -347,7 +343,6 @@ impl<'a> TableScanBuilder<'a> { plan_context: Some(plan_context), concurrency_limit_manifest_entries: self.concurrency_limit_manifest_entries, concurrency_limit_manifest_files: self.concurrency_limit_manifest_files, - concurrency_limit_data_files: self.concurrency_limit_data_files, runtime: self.table.runtime().clone(), }) } @@ -365,7 +360,6 @@ pub struct TableScan { /// The maximum number of manifest files that will be /// retrieved from [`FileIO`] concurrently concurrency_limit_manifest_files: usize, - concurrency_limit_data_files: usize, /// The maximum number of [`ManifestEntry`]s that will /// be processed in parallel @@ -516,14 +510,42 @@ impl TableScan { Ok(planned_file_rx.boxed()) } - /// Returns an [`ArrowRecordBatchStream`]. + /// Returns an [`ArrowRecordBatchStream`] using the supplied reader settings. /// - /// An unset reader concurrency inherits the scan's data-file limit; an explicit - /// reader limit overrides it. Zero reader concurrency is treated as one. + /// Reader settings are independent of file planning. Unset data-file concurrency + /// uses execution-host parallelism; zero is treated as one. + /// [`TableScanBuilder::with_concurrency_limit`] only controls manifest planning. + /// + /// ```no_run + /// # use iceberg::{Result, arrow::{ArrowReaderOptions, ParquetReadOptions}, table::Table}; + /// # async fn read(table: &Table) -> Result<()> { + /// let options = ArrowReaderOptions::builder() + /// .with_batch_size(1024) + /// .with_row_selection_enabled(true) + /// .with_parquet_options( + /// ParquetReadOptions::builder() + /// .with_metadata_size_hint(512 * 1024) + /// .build(), + /// ) + /// .build(); + /// let stream = table.scan().build()?.to_arrow(options).await?; + /// # Ok(()) + /// # } + /// ``` + /// + /// When upgrading, move reader-setting calls from `TableScanBuilder` to + /// `ArrowReaderOptions::builder()` and pass the result to `to_arrow(options)`. + /// Use `to_arrow(Default::default())` for default settings. To preserve a + /// data-file concurrency bound previously set through the scan, set + /// `with_data_file_concurrency_limit(limit)` on the reader options explicitly. + /// + /// Locally constructed readers accept the same options through + /// [`ArrowReaderBuilder::with_options`], including Parquet I/O tuning. + /// This replaces the complete configuration. Configure individual settings + /// through the options builders; per-field setters on `ArrowReaderBuilder` + /// have been removed. [`ArrowReaderBuilder::with_parquet_options`] replaces + /// only the nested Parquet configuration. pub async fn to_arrow(&self, options: ArrowReaderOptions) -> Result { - // Preserve the scan's data-file concurrency bound for backwards compatibility. - let options = - options.with_data_file_concurrency_limit_if_unset(self.concurrency_limit_data_files); ArrowReaderBuilder::new(self.file_io.clone(), self.runtime.clone()) .with_options(options) .build() @@ -682,7 +704,6 @@ mod tests { ROWS_PER_FIXTURE_FILE, TableTestFixture, assert_last_updated_seq_all, }; use crate::test_utils::test_runtime; - use crate::util::available_parallelism; use crate::util::snapshot::snapshot_id_as_of_time; use crate::{ErrorKind, Runtime, TableIdent}; @@ -3208,37 +3229,6 @@ mod tests { assert!(err.to_string().contains("not found")); } - #[test] - fn test_scan_retains_data_file_concurrency() { - for fixture in [TableTestFixture::new(), TableTestFixture::new_empty()] { - let default_scan = fixture.table.scan().build().unwrap(); - let default_limit = available_parallelism().get(); - assert_eq!( - ( - default_scan.concurrency_limit_manifest_files, - default_scan.concurrency_limit_manifest_entries, - default_scan.concurrency_limit_data_files - ), - (default_limit, default_limit, default_limit), - ); - let scan = fixture - .table - .scan() - .with_concurrency_limit(3) - .with_manifest_entry_concurrency_limit(1) - .build() - .unwrap(); - assert_eq!( - ( - scan.concurrency_limit_manifest_files, - scan.concurrency_limit_manifest_entries, - scan.concurrency_limit_data_files - ), - (3, 1, 3), - ); - } - } - #[tokio::test] async fn test_scan_deadlock() { let mut fixture = TableTestFixture::new(); From 06a41ee0aa085bf4679df690c68929d013b956bc Mon Sep 17 00:00:00 2001 From: "alex.bianchi" Date: Tue, 10 Nov 2026 07:23:46 -0500 Subject: [PATCH 14/15] refactor(arrow): keep reader concurrency solely in options --- crates/iceberg/src/arrow/reader/mod.rs | 10 ---------- crates/iceberg/src/arrow/reader/pipeline.rs | 6 +++++- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/crates/iceberg/src/arrow/reader/mod.rs b/crates/iceberg/src/arrow/reader/mod.rs index b2f66b61c8..be1f801662 100644 --- a/crates/iceberg/src/arrow/reader/mod.rs +++ b/crates/iceberg/src/arrow/reader/mod.rs @@ -96,7 +96,6 @@ impl ArrowReaderBuilder { concurrency_limit_data_files, self.runtime.clone(), ), - concurrency_limit_data_files, runtime: self.runtime.clone(), } } @@ -109,9 +108,6 @@ pub struct ArrowReader { file_io: FileIO, delete_file_loader: CachingDeleteFileLoader, - /// Effective concurrency after resolving the host default and normalizing zero. - concurrency_limit_data_files: usize, - runtime: Runtime, } @@ -133,7 +129,6 @@ mod tests { let reader = ArrowReaderBuilder::new(FileIO::new_with_memory(), test_runtime()) .with_options(options) .build(); - assert_eq!(reader.concurrency_limit_data_files, 1); let batches = tokio::time::timeout( Duration::from_secs(1), reader @@ -173,10 +168,6 @@ mod tests { .with_data_file_concurrency_limit(available_parallelism().get()) .build() ); - assert_eq!( - reader.concurrency_limit_data_files, - available_parallelism().get() - ); } #[test] @@ -198,6 +189,5 @@ mod tests { .with_options(options.clone()) .build(); assert_eq!(reader.options, options); - assert_eq!(reader.concurrency_limit_data_files, 3); } } diff --git a/crates/iceberg/src/arrow/reader/pipeline.rs b/crates/iceberg/src/arrow/reader/pipeline.rs index f011927de1..30864a02f8 100644 --- a/crates/iceberg/src/arrow/reader/pipeline.rs +++ b/crates/iceberg/src/arrow/reader/pipeline.rs @@ -65,7 +65,11 @@ impl ArrowReader { /// Take a stream of FileScanTasks and reads all the files. /// Returns a [`ScanResult`] containing the record batch stream and scan metrics. pub fn read(self, tasks: FileScanTaskStream) -> Result { - let concurrency_limit_data_files = self.concurrency_limit_data_files; + let concurrency_limit_data_files = self + .options + .data_file_concurrency_limit() + .expect("reader construction sets concurrency") + .max(1); let scan_metrics = ScanMetrics::new(); let task_reader = FileScanTaskReader { From f951b7cd5ebd31d7b974d2123460021b9d264469 Mon Sep 17 00:00:00 2001 From: alexanderbianchi <75697973+alexanderbianchi@users.noreply.github.com> Date: Wed, 11 Nov 2026 08:34:06 -0500 Subject: [PATCH 15/15] Update crates/iceberg/src/arrow/reader/mod.rs Co-authored-by: blackmwk <257669749+blackmwk@users.noreply.github.com> --- crates/iceberg/src/arrow/reader/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/iceberg/src/arrow/reader/mod.rs b/crates/iceberg/src/arrow/reader/mod.rs index be1f801662..bb0c0e8642 100644 --- a/crates/iceberg/src/arrow/reader/mod.rs +++ b/crates/iceberg/src/arrow/reader/mod.rs @@ -83,7 +83,7 @@ impl ArrowReaderBuilder { pub fn build(self) -> ArrowReader { let options = self .options - .with_data_file_concurrency_limit_if_unset(available_parallelism().get()); + .with_data_file_concurrency_limit_if_unset(available_parallelism().get().max(1)); let concurrency_limit_data_files = options .data_file_concurrency_limit() .expect("reader concurrency was set above")