You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "timsaucer (via GitHub)" <gi...@apache.org> on 2026/04/20 12:33:13 UTC

Re: [PR] feat(datafusion): enable parallel file-level scanning via one partition per file [iceberg-rust]

timsaucer commented on code in PR #2298:
URL: https://github.com/apache/iceberg-rust/pull/2298#discussion_r3110577898


##########
crates/integrations/datafusion/src/physical_plan/partitioned_scan.rs:
##########
@@ -0,0 +1,171 @@
+// 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::any::Any;
+use std::sync::Arc;
+
+use datafusion::arrow::datatypes::SchemaRef as ArrowSchemaRef;
+use datafusion::error::Result as DFResult;
+use datafusion::execution::{SendableRecordBatchStream, TaskContext};
+use datafusion::physical_expr::EquivalenceProperties;
+use datafusion::physical_plan::execution_plan::{Boundedness, EmissionType};
+use datafusion::physical_plan::stream::RecordBatchStreamAdapter;
+use datafusion::physical_plan::{DisplayAs, ExecutionPlan, Partitioning, PlanProperties};
+use futures::TryStreamExt;
+use iceberg::arrow::ArrowReaderBuilder;
+use iceberg::io::FileIO;
+use iceberg::scan::FileScanTask;
+
+use crate::to_datafusion_error;
+
+/// A DataFusion [`ExecutionPlan`] that reads one [`FileScanTask`] per partition.
+///
+/// Display information (projection, predicate) is derived at runtime from the output schema and
+/// the tasks rather than stored as dedicated struct fields.
+#[derive(Debug, Clone)]
+pub struct IcebergPartitionedScan {
+    tasks: Vec<FileScanTask>,
+    file_io: FileIO,
+    plan_properties: Arc<PlanProperties>,
+}
+
+impl IcebergPartitionedScan {
+    pub fn new(tasks: Vec<FileScanTask>, file_io: FileIO, schema: ArrowSchemaRef) -> Self {
+        let n_partitions = tasks.len();
+        let plan_properties = Self::compute_properties(schema, n_partitions);
+        Self {
+            tasks,
+            file_io,
+            plan_properties,
+        }
+    }
+
+    pub fn tasks(&self) -> &[FileScanTask] {
+        &self.tasks
+    }
+
+    pub fn file_io(&self) -> &FileIO {
+        &self.file_io
+    }
+
+    fn compute_properties(schema: ArrowSchemaRef, n_partitions: usize) -> Arc<PlanProperties> {
+        Arc::new(PlanProperties::new(
+            EquivalenceProperties::new(schema),
+            Partitioning::UnknownPartitioning(n_partitions),
+            EmissionType::Incremental,
+            Boundedness::Bounded,
+        ))
+    }
+}
+
+impl ExecutionPlan for IcebergPartitionedScan {
+    fn name(&self) -> &str {
+        "IcebergPartitionedScan"
+    }
+
+    fn as_any(&self) -> &dyn Any {
+        self
+    }
+
+    fn children(&self) -> Vec<&Arc<dyn ExecutionPlan + 'static>> {
+        vec![]
+    }
+
+    fn with_new_children(
+        self: Arc<Self>,
+        _children: Vec<Arc<dyn ExecutionPlan>>,
+    ) -> DFResult<Arc<dyn ExecutionPlan>> {
+        Ok(self)

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



##########
crates/integrations/datafusion/src/table/mod.rs:
##########
@@ -17,15 +17,20 @@
 
 //! Iceberg table providers for DataFusion.
 //!
-//! This module provides two table provider implementations:
+//! This module provides three table provider implementations:

Review Comment:
   ```suggestion
   //! This module provides various table provider implementations:
   ```



##########
crates/integrations/datafusion/src/table/partitioned.rs:
##########
@@ -0,0 +1,315 @@
+// 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::any::Any;
+use std::sync::Arc;
+
+use async_trait::async_trait;
+use datafusion::arrow::datatypes::SchemaRef as ArrowSchemaRef;
+use datafusion::catalog::Session;
+use datafusion::common::DataFusionError;
+use datafusion::datasource::{TableProvider, TableType};
+use datafusion::error::Result as DFResult;
+use datafusion::logical_expr::{Expr, TableProviderFilterPushDown};
+use datafusion::physical_plan::ExecutionPlan;
+use futures::TryStreamExt;
+use iceberg::arrow::schema_to_arrow_schema;
+use iceberg::{Catalog, Error, ErrorKind, NamespaceIdent, Result, TableIdent};
+
+use crate::error::to_datafusion_error;
+use crate::physical_plan::expr_to_predicate::convert_filters_to_predicate;
+use crate::physical_plan::partitioned_scan::IcebergPartitionedScan;
+
+/// Catalog-backed table provider that scans each data file in a separate DataFusion partition.
+///
+/// This provider reloads table metadata from the catalog on every [`scan`][Self::scan] call
+/// to guarantee freshness, then issues one DataFusion partition per data file so that
+/// DataFusion's scheduler can execute file reads in parallel.
+///
+/// Write operations are not supported. Use [`IcebergTableProvider`] for write access.
+///
+/// For consistent read-only access to a fixed snapshot without per-scan catalog overhead,
+/// use [`IcebergStaticTableProvider`] instead.
+#[derive(Debug, Clone)]
+pub struct IcebergPartitionedTableProvider {
+    catalog: Arc<dyn Catalog>,
+    table_ident: TableIdent,
+    schema: ArrowSchemaRef,
+}
+
+impl IcebergPartitionedTableProvider {
+    pub async fn try_new(
+        catalog: Arc<dyn Catalog>,
+        namespace: NamespaceIdent,
+        name: impl Into<String>,
+    ) -> Result<Self> {
+        let table_ident = TableIdent::new(namespace, name.into());
+        // First load: used only to snapshot the Arrow schema for DataFusion planning.
+        // A second load_table is issued at scan time to guarantee the freshest snapshot.
+        let table = catalog.load_table(&table_ident).await?;
+        let schema = Arc::new(schema_to_arrow_schema(table.metadata().current_schema())?);
+        Ok(Self {
+            catalog,
+            table_ident,
+            schema,
+        })
+    }
+}
+
+#[async_trait]
+impl TableProvider for IcebergPartitionedTableProvider {
+    fn as_any(&self) -> &dyn Any {
+        self
+    }
+
+    fn schema(&self) -> ArrowSchemaRef {
+        self.schema.clone()
+    }
+
+    fn table_type(&self) -> TableType {
+        TableType::Base
+    }
+
+    async fn scan(
+        &self,
+        _state: &dyn Session,
+        projection: Option<&Vec<usize>>,
+        filters: &[Expr],
+        _limit: Option<usize>,
+    ) -> DFResult<Arc<dyn ExecutionPlan>> {
+        // Per-partition row limits are not yet implemented for IcebergPartitionedScan.
+        // DataFusion will apply a GlobalLimitExec on top of this node when needed.
+
+        // Second load: fetch the latest snapshot so scans always reflect current table state.
+        let table = self
+            .catalog
+            .load_table(&self.table_ident)
+            .await
+            .map_err(to_datafusion_error)?;
+
+        // Projection indices are resolved against self.schema (captured at try_new time),
+        // same as IcebergTableProvider / IcebergTableScan.
+        let col_names = projection.map(|indices| {
+            indices
+                .iter()
+                .map(|&i| self.schema.field(i).name().clone())
+                .collect::<Vec<_>>()
+        });
+
+        let predicate = convert_filters_to_predicate(filters);
+
+        let mut builder = table.scan();
+        builder = match col_names {
+            Some(names) => builder.select(names),
+            None => builder.select_all(),
+        };
+        if let Some(pred) = predicate {
+            builder = builder.with_filter(pred);
+        }
+
+        let tasks = builder
+            .build()
+            .map_err(to_datafusion_error)?
+            .plan_files()
+            .await
+            .map_err(to_datafusion_error)?
+            .try_collect::<Vec<_>>()
+            .await
+            .map_err(to_datafusion_error)?;
+
+        let output_schema = match projection {
+            None => self.schema.clone(),
+            Some(indices) => Arc::new(self.schema.project(indices).map_err(|e| {
+                DataFusionError::Internal(format!("schema projection failed: {e}"))
+            })?),
+        };
+
+        Ok(Arc::new(IcebergPartitionedScan::new(
+            tasks,
+            table.file_io().clone(),
+            output_schema,
+        )))
+    }
+
+    fn supports_filters_pushdown(
+        &self,
+        filters: &[&Expr],
+    ) -> DFResult<Vec<TableProviderFilterPushDown>> {
+        Ok(vec![TableProviderFilterPushDown::Inexact; filters.len()])

Review Comment:
   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.



##########
crates/integrations/datafusion/src/table/partitioned.rs:
##########
@@ -0,0 +1,315 @@
+// 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::any::Any;
+use std::sync::Arc;
+
+use async_trait::async_trait;
+use datafusion::arrow::datatypes::SchemaRef as ArrowSchemaRef;
+use datafusion::catalog::Session;
+use datafusion::common::DataFusionError;
+use datafusion::datasource::{TableProvider, TableType};
+use datafusion::error::Result as DFResult;
+use datafusion::logical_expr::{Expr, TableProviderFilterPushDown};
+use datafusion::physical_plan::ExecutionPlan;
+use futures::TryStreamExt;
+use iceberg::arrow::schema_to_arrow_schema;
+use iceberg::{Catalog, Error, ErrorKind, NamespaceIdent, Result, TableIdent};
+
+use crate::error::to_datafusion_error;
+use crate::physical_plan::expr_to_predicate::convert_filters_to_predicate;
+use crate::physical_plan::partitioned_scan::IcebergPartitionedScan;
+
+/// Catalog-backed table provider that scans each data file in a separate DataFusion partition.
+///
+/// This provider reloads table metadata from the catalog on every [`scan`][Self::scan] call
+/// to guarantee freshness, then issues one DataFusion partition per data file so that
+/// DataFusion's scheduler can execute file reads in parallel.
+///
+/// Write operations are not supported. Use [`IcebergTableProvider`] for write access.
+///
+/// For consistent read-only access to a fixed snapshot without per-scan catalog overhead,
+/// use [`IcebergStaticTableProvider`] instead.
+#[derive(Debug, Clone)]
+pub struct IcebergPartitionedTableProvider {
+    catalog: Arc<dyn Catalog>,
+    table_ident: TableIdent,
+    schema: ArrowSchemaRef,
+}
+
+impl IcebergPartitionedTableProvider {
+    pub async fn try_new(
+        catalog: Arc<dyn Catalog>,
+        namespace: NamespaceIdent,
+        name: impl Into<String>,
+    ) -> Result<Self> {
+        let table_ident = TableIdent::new(namespace, name.into());
+        // First load: used only to snapshot the Arrow schema for DataFusion planning.
+        // A second load_table is issued at scan time to guarantee the freshest snapshot.
+        let table = catalog.load_table(&table_ident).await?;
+        let schema = Arc::new(schema_to_arrow_schema(table.metadata().current_schema())?);
+        Ok(Self {
+            catalog,
+            table_ident,
+            schema,
+        })
+    }
+}
+
+#[async_trait]
+impl TableProvider for IcebergPartitionedTableProvider {
+    fn as_any(&self) -> &dyn Any {
+        self
+    }
+
+    fn schema(&self) -> ArrowSchemaRef {
+        self.schema.clone()
+    }
+
+    fn table_type(&self) -> TableType {
+        TableType::Base
+    }
+
+    async fn scan(
+        &self,
+        _state: &dyn Session,
+        projection: Option<&Vec<usize>>,
+        filters: &[Expr],
+        _limit: Option<usize>,
+    ) -> DFResult<Arc<dyn ExecutionPlan>> {
+        // Per-partition row limits are not yet implemented for IcebergPartitionedScan.
+        // DataFusion will apply a GlobalLimitExec on top of this node when needed.
+
+        // Second load: fetch the latest snapshot so scans always reflect current table state.
+        let table = self
+            .catalog
+            .load_table(&self.table_ident)
+            .await
+            .map_err(to_datafusion_error)?;
+
+        // Projection indices are resolved against self.schema (captured at try_new time),
+        // same as IcebergTableProvider / IcebergTableScan.
+        let col_names = projection.map(|indices| {
+            indices
+                .iter()
+                .map(|&i| self.schema.field(i).name().clone())
+                .collect::<Vec<_>>()
+        });
+
+        let predicate = convert_filters_to_predicate(filters);
+
+        let mut builder = table.scan();
+        builder = match col_names {
+            Some(names) => builder.select(names),
+            None => builder.select_all(),
+        };
+        if let Some(pred) = predicate {
+            builder = builder.with_filter(pred);
+        }
+
+        let tasks = builder
+            .build()
+            .map_err(to_datafusion_error)?
+            .plan_files()
+            .await
+            .map_err(to_datafusion_error)?
+            .try_collect::<Vec<_>>()
+            .await
+            .map_err(to_datafusion_error)?;

Review Comment:
   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? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org