feat(scan): expose portable reader options for planned tasks by alexanderbianchi · Pull Request #3514 · apache/iceberg-rust

alexanderbianchi · 2026-10-22T19:06:59Z

Which issue does this PR close?

Follows up on the reader-options discussion in #2671 and supports apache/datafusion-iceberg#67. Does not close #2671; the DataFusion integration continues in its own repository.

What changes are included in this PR?

Separate reader settings from file planning with a serializable ArrowReaderOptions value. TableScan::to_arrow(options) and ArrowReaderBuilder::with_options(options) use the same mapping, letting eager consumers retain tasks and settings, discard the scan, and construct readers with local I/O and runtime resources.

let options = ArrowReaderOptions::builder()
    .with_batch_size(Some(1024))
    .build();
let tasks: Vec<_> = scan.plan_files().await?.try_collect().await?;
drop(scan);

let reader = ArrowReaderBuilder::new(local_file_io, local_runtime)
    .with_options(options)
    .build();

Options cover batch size, data-file concurrency, row-group filtering, row selection, and bloom filtering. Unset concurrency resolves on the execution host when the reader is built. Fields are private, with a typed builder and serde defaults for missing fields. Local Parquet I/O tuning survives applying the options. Clone one reader across partitions to share delete loading.

API migration: move reader-setting calls from TableScanBuilder to ArrowReaderOptions::builder(), then pass the result to to_arrow(options). For defaults, use to_arrow(Default::default()). Scan with_concurrency_limit() now controls manifest planning only; configure with_data_file_concurrency_limit(Some(limit)) on reader options to also limit data reads.

This enables #67 to preserve non-default reader settings in a follow-up, without adding an eager planner, a DataFusion dependency, or changes to the task wire format.

Are these changes tested?

Regression coverage serializes settings, drops the scan, and reads filtered tasks with position deletes using fresh local I/O/runtime. It compares results with to_arrow(options) and checks batch sizing. Focused tests cover default decoding, unknown fields, explicit settings, execution-host defaults, and preservation of Parquet I/O tuning.

Passed: 2,032 library tests (one existing ignored test), 87 documentation tests, Clippy with all targets/features and -D warnings, formatting, and git diff --check. Compiled all targets in the affected examples, S3 Tables catalog, and integration-test crates; external-service integration tests were not executed. Regenerated and inspected iceberg/public-api.txt.

AI Disclosure

Implemented with Codex assistance. Review focus: migration from scan configuration to reader options, serialized-option evolution, and resolving unset concurrency on the execution host.

alexanderbianchi · 2026-10-22T19:17:59Z

@laskoviymishka

comphead

Thanks for the PR.

The apply_to mapping matches the old to_arrow() wiring (an unset batch size still falls back to the reader default). Four comments inline.

comphead · 2026-10-22T20:31:51Z

+/// 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)]


On the serialized-option evolution you asked about: every field is required on deserialize and the struct is Copy + Eq. A payload written by an older planner will fail with missing field on a newer worker once a sixth setting is added. A non-Copy field would also break the public API.

Consider #[serde(default)] (container level, with a Default that matches the ArrowReaderBuilder defaults) so new fields can be added later. Unknown fields from a newer planner are already ignored by serde.

Question (unverified): data_file_concurrency_limit defaults to available_parallelism() of the planning host and is now serialized. A coordinator with 64 cores would pass 64 to a 4-core worker unless the caller overrides it. For comparison, Java's SparkInputPartition ships caseSensitive, the expected schema, and cacheDeleteFilesOnExecutors, but no batch size or concurrency. Should this field be Option so an unset value keeps the local default, like batch_size?

comphead · 2026-10-22T20:31:51Z

+
+impl TableScanReaderOptions {
+    /// The requested record batch size, or the reader default.
+    pub fn batch_size(&self) -> Option<usize> {


The five getters have no caller in this PR and the description only needs apply_to plus serde. Each public getter is API to keep stable. Consider dropping them until a consumer needs one, which also shrinks public-api.txt.

comphead · 2026-10-22T20:31:51Z

+    /// # Ok(())
+    /// # }
+    /// ```
+    pub fn reader_options(&self) -> TableScanReaderOptions {


The same five settings now live in TableScanBuilder, TableScan (two struct literals in build()), ArrowReaderBuilder, and this new type, and reader_options() copies them one by one. If it stays simple, store a TableScanReaderOptions in TableScanBuilder and TableScan with the with_* setters writing into it. Then reader_options() returns self.reader_options and both TableScan { .. } literals lose five fields each. Skip if the setter churn outweighs that.

No, we should not add this method, and we should remove all methods used to config arrow reader options.

comphead · 2026-10-22T20:31:51Z

+    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] {


The for enabled in [false, true] loop does not add coverage for the flags. Mutation check: deleting the row_selection_enabled and bloom_filter_enabled assignments in apply_to fails only applies_scan_settings_and_preserves_local_io_tuning. This test still passes because the result equals to_arrow() either way. Consider a single iteration and keep what only this test covers: the JSON shape, drop-scan, fresh local I/O, and the batch size bound.

blackmwk

Thanks @alexanderbianchi for this pr, I have some opnions about the api design

blackmwk · 2026-10-23T10:23:28Z

+/// 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 {


We should derive typed buidler rather than make field crate visible, all fields should be private.

blackmwk · 2026-10-23T10:29:51Z

Reanme this to options.

https://github.com/alexanderbianchi/iceberg-rust/blob/e9d2a8a99399ab0440402c5935e37f797cbae60d/crates/iceberg/src/arrow/reader/options.rs there is an options.rs already. Should we combine them? Move options.rs to parquet_options.rs?

blackmwk · 2026-10-23T10:30:23Z

+/// 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 {


Suggested change

pub struct TableScanReaderOptions {

pub struct ArrowReaderOptions {

blackmwk · 2026-10-23T10:31:13Z

@@ -602,23 +602,51 @@ impl TableScan {

    /// Returns an [`ArrowRecordBatchStream`].
    pub async fn to_arrow(&self) -> Result<ArrowRecordBatchStream> {


Suggested change

pub async fn to_arrow(&self) -> Result<ArrowRecordBatchStream> {

pub async fn to_arrow(&self, options: ArrowReaderOptions) -> Result<ArrowRecordBatchStream> {

blackmwk · 2026-10-23T10:31:23Z

+    /// # Ok(())
+    /// # }
+    /// ```
+    pub fn reader_options(&self) -> TableScanReaderOptions {


No, we should not add this method, and we should remove all methods used to config arrow reader options.

blackmwk · 2026-10-23T10:32:40Z

+
+    /// 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 {


We should not add this, instead, we should add a with_options methods in ArrowReaderBuidler and pass in this struct.

feat(scan): expose portable reader options for planned tasks

e9d2a8a

github-actions Bot added the datafusion label Oct 22, 2026

comphead reviewed Oct 22, 2026

View reviewed changes

blackmwk requested changes Oct 23, 2026

View reviewed changes

refactor(arrow): separate reader options from scan planning

2fe0018

laskoviymishka self-requested a review October 23, 2026 19:50

alexanderbianchi mentioned this pull request Oct 23, 2026

docs: propose composable Iceberg file reads alexanderbianchi/iceberg-rust#5

Draft

	pub struct TableScanReaderOptions {
	pub struct ArrowReaderOptions {

		@@ -602,23 +602,51 @@ impl TableScan {

		/// Returns an [`ArrowRecordBatchStream`].
		pub async fn to_arrow(&self) -> Result<ArrowRecordBatchStream> {

	pub async fn to_arrow(&self) -> Result<ArrowRecordBatchStream> {
	pub async fn to_arrow(&self, options: ArrowReaderOptions) -> Result<ArrowRecordBatchStream> {

Conversation

alexanderbianchi commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

What changes are included in this PR?

Are these changes tested?

AI Disclosure

Uh oh!

alexanderbianchi commented Oct 22, 2026

Uh oh!

comphead left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blackmwk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alexanderbianchi commented Oct 22, 2026 •

edited

Loading

comphead left a comment •

edited

Loading