Unify how various `FileSource`s are applying projections?
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
Related to https://github.com/apache/datafusion/pull/17076
_tl;dr: file sources are applying projections in various areas. My proposal is to centralize this into one area. This way, we can avoid passing `FileScanConfig` in various places`_
Hi, I'm curious about the circular relationship between `FileScanConfig` and `FileSource`. Specifically, how various file sources apply projection when creating a file opener.
When we create a file stream in `FileScanConfig`, we do the following: https://github.com/apache/datafusion/blob/173989cc2fb55c30cd174b520754812ea408e00b/datafusion/datasource/src/file_scan_config.rs#L507-L514
`FileScanConfig` has an inner field of `FileSource`, and a `FileSource` creates a file opener. In both of these steps, we pass in `FileScanConfig`, which feels a bit weird to me. (1. `with_projection(self)`, and 2. `create_file_opener(..., self, ...)`.
Looking through the various implementation of `with_projection`, we see some file sources do nothing other than do a deep clone:
`ParquetSource`
https://github.com/apache/datafusion/blob/173989cc2fb55c30cd174b520754812ea408e00b/datafusion/datasource-parquet/src/source.rs#L575-L577
`JsonSource`
https://github.com/apache/datafusion/blob/173989cc2fb55c30cd174b520754812ea408e00b/datafusion/datasource-json/src/source.rs#L135-L137
It's not that they don't need to project columns, but the actual projection for these file sources occur in `create_file_opener`, hence why we pass in the `FileScanConfig` again.
`ParquetSource`
https://github.com/apache/datafusion/blob/173989cc2fb55c30cd174b520754812ea408e00b/datafusion/datasource-parquet/src/source.rs#L461-L470
`JsonSource`
https://github.com/apache/datafusion/blob/173989cc2fb55c30cd174b520754812ea408e00b/datafusion/datasource-json/src/source.rs#L99-L114
For the other file sources, the projection occurs within `with_projection`:
`CsvSource`
https://github.com/apache/datafusion/blob/173989cc2fb55c30cd174b520754812ea408e00b/datafusion/datasource-csv/src/source.rs#L260-L264
`AvroSource`
https://github.com/apache/datafusion/blob/173989cc2fb55c30cd174b520754812ea408e00b/datafusion/datasource-avro/src/source.rs#L98-L102
It would be nice to unify this logic, and make the design/relationship between `FileSource` and `FileScanConfig` much simpler. One idea I had was to move the explicit projection into the `create_file_opener` impls, removing the need to call `.with_projection`. WIth https://github.com/apache/datafusion/pull/17076, we'd just directly call `file_source.create_file_opener`.
Contributor guide
Research direction
Start with datafusion/datasource/src/file_scan_config.rs and compare FileSource implementations in the Parquet, JSON, CSV, and Avro source files, focusing on with_projection and create_file_opener. Review the related pull request, then define a unified projection path that removes the inconsistent FileScanConfig relationship and verify the affected file sources still apply projections correctly.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend-api-design, data-engineering
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100