Let `FileScanConfig` own a list of `ProjectionExpr`s #18253

friendlymatthew · 2025-10-23T19:59:08Z

Which issue does this PR close?

Related to Support Push down expression evaluation in TableProviders #14993

Rationale for this change

To enable expression pushdown to file sources, we need to plumb expressions through the FileScanConfig layer. Currently, FileScanConfig only tracks column indices for projection, which limits us to simple and naive column selection.

This PR begins expression pushdown implementation by having FileScanConfig own a list of ProjectionExprs, instead of column indices. This allows file sources to eventually receive and optimize based on the actual expressions being projected.

Notes about this PR

The first commit is based off of Use TableSchema in FileScanConfig #18231
To avoid a super large diff and a harder review, I've decided to break (Support Push down expression evaluation in TableProviders #14993) into 2 tasks:
- Have the DataSource (FileScanConfig) actually hold projection expressions (this PR)
- Flow the projection expressions from DataSourceExec all the way to the FileSource

adriangb

This generally looks great @friendlymatthew ! Let's have one more reviewer go through it though.

adriangb · 2025-10-27T01:40:48Z

datafusion/physical-expr/src/projection.rs

 /// methods to manipulate and analyze the projection as a whole.
 #[derive(Debug, Clone)]
-pub struct Projection {
+pub struct ProjectionExprs {


I do think this name is better and it's not a breaking change since Projection was introduced after the last release. To make sure we get this through as fast as possible (in particular before it does become a breaking change) could you make this it's own PR?

adriangb · 2025-10-27T01:41:49Z

datafusion/datasource/src/file_scan_config.rs

+        let projection = projection_indices.as_ref().map(|indices| {
+            ProjectionExprs::from_indices(indices, table_schema.table_schema())
+        });


Note: this is because we are not changing FileScanConfigBuilder. I think it makes sense to change it at some point but it's not necessary yet and we can keep things as backwards compatible as possible for as long as possible.

Should we deprecate with_projection and add a new function with_projection_indices to help downstream users prepare for the change?

That's a good question. The main place where this API is used by actual users is in TableProvider::scan_with_args. The projection pushed down into TableProvider is a Vec<usize> so that's all you have. I'd argue that we should change that so that the full expression tree gets pushed down there as well. But for now we can either:

Put a convenience method on FileScanConfigBuilder like you say.

Make users call ProjectionExprs::from_indices(&projection, &schema) or something like that which seems like it isn't too bad and keeps us from adding a method we know we probably want to remove later.

adriangb · 2025-10-27T01:42:44Z

datafusion/datasource/src/file_scan_config.rs

-            Some(proj) => proj.clone(),
+            Some(proj) => proj.ordered_column_indices(),


I similarly think long term we want to get rid of projection_indices and some of these other helper functions (a lot of it drops away once we have the projection expressions because we can re-use the same machinery that ProjectionExec uses) but for now we do what we can to keep it backwards compatible and minimize churn.

adriangb · 2025-10-27T01:43:19Z

datafusion/physical-expr/src/projection.rs

+        }
+    }
+
+    pub fn from_indices(indices: &[usize], schema: &SchemaRef) -> Self {


Please add documentation, including handling of duplicates, ordering, etc.

adriangb · 2025-10-27T01:44:20Z

datafusion/physical-expr/src/projection.rs

            .collect_vec()
    }

+    /// Extract the ordered column indices for a column-only projection.


Please add more detailed documentation, e.g. what happens if the projection contains non-column expressions, what if the column expressions are nested within other expressions, etc.

alamb

Looks good to me too -- thank you @friendlymatthew

Let's address @adriangb 's comments and get this merged in

alamb · 2025-10-27T12:44:46Z

datafusion/datasource/src/file_scan_config.rs

+    /// Each expression in the projection can reference columns from both the file
+    /// schema and table partition columns. If `None`, all columns from the table
+    /// schema are projected.
+    pub projection: Option<ProjectionExprs>,


since this is a pub field, I think it qualifies as an API change so I marked the PR as such

In that case, I'll rename the field to indicate it holds a list of projections

alamb · 2025-10-27T12:46:15Z

datafusion/datasource/src/file_scan_config.rs

+        let projection = projection_indices.as_ref().map(|indices| {
+            ProjectionExprs::from_indices(indices, table_schema.table_schema())
+        });


Should we deprecate with_projection and add a new function with_projection_indices to help downstream users prepare for the change?

adriangb · 2025-10-27T17:28:00Z

datafusion/datasource/src/file_scan_config.rs

+    /// # Deprecated
+    /// Use [`Self::with_projection_indices`] instead. This method will be removed in a future release.
+    #[deprecated(since = "51.0.0", note = "Use with_projection_indices instead")]
+    pub fn with_projection(self, indices: Option<Vec<usize>>) -> Self {
+        self.with_projection_indices(indices)
+    }
+
+    /// Set the columns on which to project the data using column indices.
+    ///
+    /// Indexes that are higher than the number of columns of `file_schema` refer to `table_partition_cols`.
+    pub fn with_projection_indices(mut self, indices: Option<Vec<usize>>) -> Self {


Is the idea here to "reserve" the with_projection API to accept a &ProjectionExprs later on? Or could you elaborate on the long term vision behind the name change / deprecation?

Is the idea here to "reserve" the with_projection API to accept a &ProjectionExprs later on?

No, I'm planning to add with_projection_exprs. I just haven't wired it up yet-- I'm working through this incrementally to fix the test breakages first

Okay makes sense, followup it is then!

alamb · 2025-10-27T20:46:27Z

🚀

## Which issue does this PR close? - Related to apache#14993 ## Rationale for this change To enable expression pushdown to file sources, we need to plumb expressions through the `FileScanConfig` layer. Currently, `FileScanConfig` only tracks column indices for projection, which limits us to simple and naive column selection. This PR begins expression pushdown implementation by having `FileScanConfig` own a list of `ProjectionExpr`s, instead of column indices. This allows file sources to eventually receive and optimize based on the actual expressions being projected. ## Notes about this PR - The first commit is based off of apache#18231 - To avoid a super large diff and a harder review, I've decided to break (apache#14993) into 2 tasks: - Have the `DataSource` (`FileScanConfig`) actually hold projection expressions (this PR) - Flow the projection expressions from `DataSourceExec` all the way to the `FileSource` --------- Co-authored-by: Adrian Garcia Badaracco <[email protected]>

adriangb and others added 3 commits October 23, 2025 15:36

Have FileScanConfig own table_schema

7441afe

Nit: rename Projection -> ProjectionExprs

2621f4e

Have FileScanConfig own a ProjectionExprs

2d1ad86

github-actions bot added physical-expr Changes to the physical-expr crates core Core DataFusion crate substrait Changes to the substrait crate proto Related to proto crate datasource Changes to the datasource crate physical-plan Changes to the physical-plan crate labels Oct 23, 2025

friendlymatthew force-pushed the friendlymatthew/pushdown-propogation branch 3 times, most recently from 01f0446 to 248d1b9 Compare October 23, 2025 20:49

Fix documentation, typos, clippy

9e4344a

friendlymatthew force-pushed the friendlymatthew/pushdown-propogation branch from 248d1b9 to 9e4344a Compare October 23, 2025 23:14

Merge branch 'main' into friendlymatthew/pushdown-propogation

eec8ec6

adriangb approved these changes Oct 27, 2025

View reviewed changes

adriangb mentioned this pull request Oct 27, 2025

Support Push down expression evaluation in TableProviders #14993

Closed

alamb added the api change Changes the API exposed to users of the crate label Oct 27, 2025

alamb approved these changes Oct 27, 2025

View reviewed changes

friendlymatthew added 2 commits October 27, 2025 11:20

Rename FileScanConfig::projection -> FileScanConfig::projection_exprs

e20b1a0

Add comment for ProjectionExpr::from_indices

f5091c3

friendlymatthew force-pushed the friendlymatthew/pushdown-propogation branch from 207316d to f5091c3 Compare October 27, 2025 15:40

Add comments for ProjectionExprs::ordered_column_indices

5951301

github-actions bot added the catalog Related to the catalog crate label Oct 27, 2025

Deprecate with_projection

6d6776a

friendlymatthew force-pushed the friendlymatthew/pushdown-propogation branch from eeb2a7f to 6d6776a Compare October 27, 2025 16:44

github-actions bot added the documentation Improvements or additions to documentation label Oct 27, 2025

Add notes to upgrading guide

8eedb93

friendlymatthew force-pushed the friendlymatthew/pushdown-propogation branch from e93cdb0 to 8eedb93 Compare October 27, 2025 17:02

Merge branch 'main' into friendlymatthew/pushdown-propogation

d578348

friendlymatthew force-pushed the friendlymatthew/pushdown-propogation branch from bc9e9f8 to d578348 Compare October 27, 2025 17:07

Fix up clippy

c6549dd

adriangb reviewed Oct 27, 2025

View reviewed changes

adriangb added this pull request to the merge queue Oct 27, 2025

Merged via the queue into apache:main with commit 6eb8d45 Oct 27, 2025
33 checks passed

adriangb deleted the friendlymatthew/pushdown-propogation branch October 27, 2025 18:48

friendlymatthew mentioned this pull request Oct 27, 2025

Evaluate projections at the file source level #18309

Closed

		Some(proj) => proj.clone(),
		Some(proj) => proj.ordered_column_indices(),

Let FileScanConfig own a list of ProjectionExprs #18253

Let FileScanConfig own a list of ProjectionExprs #18253

Uh oh!

Conversation

friendlymatthew commented Oct 23, 2025

Which issue does this PR close?

Rationale for this change

Notes about this PR

Uh oh!

adriangb 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!

alamb 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!

Uh oh!

alamb commented Oct 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Let `FileScanConfig` own a list of `ProjectionExpr`s #18253

Let `FileScanConfig` own a list of `ProjectionExpr`s #18253