feat!(datafusion): enable parallel file scanning with eager task bucketing

[toutane]

Which issue does this PR close?

• Closes: Enable parallel file-level scanning for IcebergTableScan Datafusion Integration #2220
• Related:
  • EPIC: Support parallel scan in iceberg-datafusion #1604 (EPIC: sub-file parallelism, long-term direction)
  • Plan file scan task according scan file size. #128 (size-based planning)

What changes are included in this PR?

Approach

Rather than introducing new types (IcebergPartitionedScan, IcebergPartitionedTableProvider as originally proposed), this PR extends the existing IcebergTableProvider / IcebergTableScan with an eager mode where file scan tasks are planned at scan() time and distributed into buckets, one bucket per DataFusion partition.

The main motivation is to let DataFusion schedule file reads concurrently. Previously all files streamed through a single partition (UnknownPartitioning(1)); now IcebergTableProvider::scan distributes tasks across min(target_partitions, n_files) partitions, and declares Partitioning::Hash when the data is identity-partitioned.

Key changes

• TableScan::to_arrow_from_tasks - New public method on TableScan that accepts a pre-collected FileScanTaskStream instead of calling plan_files() internally. This is the hook used by IcebergTableScan::execute(i) to replay each bucket through the Arrow reader while preserving all reader-side configuration (concurrency limit, row-group filtering, batch size). Tasks must come from a TableScan with the same projection and filters as self - predicates are baked into each task at planning time and are not re-applied by the reader. The doc comment makes this contract explicit.

• IcebergTableScan is now pub - Previously pub(crate). Made public so that downstream integrations that need to inspect or wrap the physical plan can do so without going through the table provider.

• with_new_children now returns an error - IcebergTableScan is a leaf node and does not support children. Previously the implementation silently dropped any children passed to it; it now returns DataFusionError::Internal when children is non-empty, matching the contract of IcebergCommitExec.

• Eager task planning in IcebergTableProvider::scan - plan_files() is now called at planning time (inside TableProvider::scan) rather than at execution time (inside ExecutionPlan::execute). The collected tasks are distributed into min(target_partitions, n_files) buckets by bucketing::bucket_tasks and stored in the scan. Each execute(i) call then fetches its pre-assigned bucket and streams it through to_arrow_from_tasks - no redundant metadata reads per partition.

• bucketing module - Handles bucket assignment and Partitioning declaration. For tables with a single partition spec using only identity transforms, tasks are hashed on their partition values using DataFusion's create_hashes + REPARTITION_RANDOM_STATE, and the scan declares Partitioning::Hash. This lets DataFusion recognize that the output is already hash-partitioned and skip a downstream RepartitionExec. Non-identity transforms (bucket, truncate, year/month/day/hour) are lossy: the partition value in task metadata does not match what DataFusion would compute by hashing the actual column values, so those cases fall back to UnknownPartitioning. Any task that cannot be fully hashed with the identity key (unsupported literal type, null partition value) also falls back.
  Credit: This bucketing solution was proposed by @timsaucer.

Design choices - planning at scan() time vs. at execute() time

Planning eagerly at scan() time is a deliberate trade-off:

• Pro: Tasks are computed once and shared across all partitions; the plan is reproducible; execute(i) is pure I/O with no catalog round-trips.
• Con: TableProvider::scan now does network I/O (catalog + metadata reads), which is unusual for a planning-phase method. An alternative design - planning lazily at execute time - would keep scan() cheap but requires one plan_files() call per partition (redundant). A future extension could expose this as an option for use cases where snapshot staleness matters more than plan reproducibility.

Known limitations

• Limited type support for Partitioning::Hash - literal_to_array supports seven primitive Arrow types (Bool, Int32, Int64, Float32, Float64, Utf8, Date32). Timestamps, Decimal128, LargeUtf8, etc. are not yet covered; any unsupported type forces fallback to UnknownPartitioning.

• Spec evolution disables Partitioning::Hash - If the table has more than one historical partition spec, the bucketing module conservatively returns UnknownPartitioning to avoid mismatches between old and new partition tuple layouts.

Are these changes tested?

Unit tests in table/mod.rs covering the new bucketed scan path:

• test_empty_table_single_empty_bucket - Empty table produces one empty bucket, guarding against out-of-bounds panic on execute(0).
• test_unpartitioned_falls_back_to_unknown - Unpartitioned table declares UnknownPartitioning.
• test_bucket_count_capped_at_file_count - When target_partitions > n_files, bucket count is capped at n_files.
• test_single_target_partition_single_bucket - target_partitions=1 produces a single bucket regardless of file count, reproducing the original single-threaded behavior.
• test_identity_partitioned_declares_hash - Identity-partitioned table declares Partitioning::Hash referencing the partition column.
• test_projection_without_partition_col_falls_back_to_unknown - Projecting away an identity column falls back to UnknownPartitioning.

Additional tests are added for IcebergTableProvider to cover limit pushdown, insert behavior, and schema consistency, ensuring the refactor introduces no regressions on existing functionality.

SQL logic tests - EXPLAIN snapshots are updated to reflect the new buckets:[N] file_count:[M] display format and the correct input_partitions counts.

Production validation - We plan to test these changes in our infrastructure by shadowing real-world queries.

Follow-up work

• Redundant FilterExec - @timsaucer reports that supports_filters_pushdown returns Inexact for all filters, causing DataFusion to insert a FilterExec above IcebergTableScan even though the Arrow reader already applies the predicate via ArrowPredicateFn. Returning Exact for losslessly-converted filters would eliminate this redundant re-evaluation.
  He proposed a solution in earlier commits, but those have been reverted as they are out of scope for this PR. This issue is tracked in IcebergTableProvider::supports_filters_pushdown marks every filter as Inexact, causing a redundant FilterExec above IcebergTableScan #2363.

Note

IcebergStaticTableProvider is unchanged - it still uses IcebergTableScan::new (lazy, single-partition). Static snapshots do not benefit from eager planning because the task list is fixed by construction.

[timsaucer]

I'm no expert on Iceberg but I've worked a lot on DataFusion, particularly table providers. I wrote a blog on the datafusion site recently, but since you first put this PR up. In case it's in any way useful: https://datafusion.apache.org/blog/2026/03/31/writing-table-providers/

Overall I think the approach here is definitely reasonable. My comments are mostly around opportunities to squeeze out a little more performance based on having done something similar at my work.

[timsaucer]

Since this doesn't support children, I'd recommend an error if _children is not empty. Not a blocker for merge.

[toutane]

Yes, you're right thanks! Pushed a fix that returns a DataFusionError::Internal, matching the pattern used in IcebergCommitExec::with_new_children.

Side note: IcebergTableScan::with_new_children has the same issue. This could be the subject of another PR.

[timsaucer]

Can we do better than this? If we have partitioned scan and the filter is on the partitions I would expect to be able to get an exact pushdown. That would entirely remove a filter operation for cases where it matches, and I think that's a big win and common use case I've seen in other work.

[toutane]

Yes, you're right there's something to do here, I agree.

I'd prefer to tackle this in a follow-up PR: doing it correctly requires a per-filter conversion API (currently convert_filters_to_predicate collapses everything into a single combined predicate and silently drops non-convertible filters) and, in a partition-spec-aware check, only Identity-transformed partition columns can be safely marked Exact; bucket, truncate, year/month/etc. are lossy and must stay Inexact to avoid incorrect results.

Happy to open a tracking issue. However, if you think it's simple enough, I can go ahead and make the changes directly in the PR.

[timsaucer]

It looks like the number of output partitions will be the number of files, right? I'm wondering if there's an opportunity to do better than that. We're specifying that the output partitioning in the exec is unknown, but don't we have information about the partitioning we could utilize?

[timsaucer]

By better I mean could we be more performant if we were to go ahead and get the target partitions from the session and output in those number of partitions already with hashing?

[toutane]

Thanks for raising this, please push back if any of the below is off.

For context, the long-term direction for this is tracked in the EPIC #1604 (row-group-based parallel scan with a GroupPruner that can split/merge FileScanTask below the file grain). What I was hoping to land with this PR is a more immediate, scoped optimization that stays within the current file-grain contract, so we don't preempt the design choices in #1604. The file-grouping step you're pointing at is essentially what #2220 describes as the intermediate improvement on the path toward #1604.

If you think it's appropriate, I'd be happy to pick up a short-term follow-up along these lines:

1. Switch IcebergPartitionedScan from tasks: Vec<FileScanTask> to file_groups: Vec<Vec<FileScanTask>>, to follow the convention used by DataFusion's own FileScanConfig, each group = one DataFusion partition that streams its files sequentially through ArrowReaderBuilder::read.
2. In IcebergPartitionedTableProvider::scan, read state.config().target_partitions() and group tasks into min(n_files, target_partitions) buckets.
3. When n_files < target_partitions, parallelism is still capped at n_files. I think that's inherent to the file grain, but let me know if I'm missing something.

I'm happy to open the follow-up issue/PR myself, or defer to you if you'd rather frame it, whatever works best.

[timsaucer]

I suppose I'd need to understand those conversations. I think I mentioned in one of the other comments on this PR, but I found the whole discussion difficult to track. Maybe I can find some time this weekend to look through that sized based partitioning they mention.

[timsaucer]

I wrote this PR targeting your branch. Let me know what you think!

toutane#1

The one issue I have is that I do not personally have access to any iceberg catalogs that I could use for benchmarking. My ability to test it is very limited right now.

[toutane]

Hey Tim, thanks a lot for the proposal. It is really clean and smart.

I created an issue for the redundant FilterExec you were mentioning (#2363), so nice that you're addressed it here.

For the benchmark, we can do it in our infra by shadowing real traffic (our ultimate goal would be to distribute execution on multiple workers, based on the output partitioning). I will not be a standard benchmark but at least it will show if things are improving on real world queries.

What do you finally thing of merging this new provider/scan with the current one so that we only maintain one path as you suggested? If I understand correctly the current path is reachable by setting target_partitions to 1.

Last thing, I'll try to support partitioning based on Iceberg bucket transform, the tricky thing being that DataFusion and Iceberg aren't using the same hash function making the bucket hash incompatible with RepartitionExec.

[timsaucer]

Personally I strongly believe you should be updating the existing table provider instead of creating a new one. I think it's just more work in the long run to keep to near identical bits of code.

I don't think you'll be able to use iceberg bucket transforms for the datafusion hashing output.

[mbutrovich]

Thanks for the PR, @toutane! One thing I noticed: IcebergPartitionedScan::execute() creates a bare ArrowReaderBuilder::new(file_io).build() with no configuration. The existing path through IcebergTableScan wires through row group filtering, row selection, concurrency limits, and batch size. Might be worth plumbing those through here too so users don't silently lose those optimizations when switching to the partitioned scan.

[timsaucer]

More broadly, is adding in a second path really the best answer? It seems like now you're going to increase your maintenance load. Is there any reason not to have a single path and the fallback be that it's a partitioned scan of N=1?

I am going to spend a little more time trying to understand the issues. It's difficult because some of them are marked as unplanned or stale and some of the links do not have good descriptions. I suppose I'll need to look at the java source to get a better idea of what the long term goal is.

[toutane]

Hey Tim, I think you're absolutely right about consolidating everything into a single TableProvider long term.

The only reason I kept separate paths was to avoid introducing breaking changes. I am going to explore a design where partitioned file scan becomes the default behavior, with the current provider's logic as a fallback as you suggested.

On a related note, it could be worth thinking about the next step: exposing Partitioning::Hash as output-partitioned when the Iceberg data uses bucket partitioning. Do you think that fits naturally in the same path, or would a separate provider be a better fit?

[timsaucer]

Hey Tim, I think you're absolutely right about consolidating everything into a single TableProvider long term.

The only reason I kept separate paths was to avoid introducing breaking changes. I am going to explore a design where partitioned file scan becomes the default behavior, with the current provider's logic as a fallback as you suggested.

On a related note, it could be worth thinking about the next step: exposing Partitioning::Hash as output-partitioned when the Iceberg data uses bucket partitioning. Do you think that fits naturally in the same path, or would a separate provider be a better fit?

I understand a desire to not introduce breaking changes. Is the concern that the API is changing or do you have implementation concerns? If it's just the API change, then I think a good upgrade documentation is often sufficient, especially since it looks like the change would be fairly straightforward for a downstream consumer. Please correct me if that's not correct.

If it's concern about the implementation, then I think the real solution is to make sure there's robust testing both in the repo and against some real life workloads to verify performance at different scales and partitioning structures.

With respect to the question about output partitioning, I think any time you can do that you should. Any time we can give more information about these kinds of things we're going to see performance gains, and sometimes significant gains.