mitchsw opened a new pull request, #23467:
URL: https://github.com/apache/datafusion/pull/23467

   ## Which issue does this PR close?
   
   - Closes #23463
   
   ## Rationale for this change
   
   `FileSource` (the per-format scan trait) is currently coupled to its 
concrete `FileScanConfig` parent. A `FileSource`'s open-time methods take a 
`&FileScanConfig` and use it as a "bag of args". That forces a custom 
`DataSource` to build a `FileScanConfig` it doesn't need just to reuse an 
existing `FileSource`. Passing those inputs explicitly in an arg struct 
decouples the two.
   
   This PR strictly layers the types:
   
   ```text
   DataSourceExec ──► DataSource (trait) ──► FileScanConfig ──► FileSource 
(trait) ──► ParquetSource / ...
   ```
   
   ## What changes are included in this PR?
   
   - Add `FileSourceArgs`, which `FileSource::create_file_opener` / 
`create_morselizer` now take instead of `FileScanConfig base_config`
   - Also pulls `object_store` and `FileSource::with_batch_size` into 
`FileSourceArgs`.
   - `FileScanConfig` builds `FileSourceArgs` inline in its open path (no-op).
   - Updates all `FileSource` implementations and the `csv_json_opener` example.
   
   ## Why also remove `with_batch_size`?
   
   Not strictly needed to close #23463 (the decoupling only needs the 
`create_*` signature change), but bundled as a cleanup:
   
   - `batch_size` is an open-time input like the others — it belongs in 
`FileSourceArgs`, not in a separate mutate-and-clone method.
   - It removes a runtime invariant: sources store `Option<usize>` and later 
`.expect("Batch size must be set …")`; a required arg is compiler-enforced.
   - The method is a single-purpose shim — one caller on `main`, and 
`ArrowSource` ignores its argument (`fn with_batch_size(&self, _batch_size: 
usize)`).
   - No extra breaking cost: the trait already changes here, so implementors 
update regardless.
   
   Happy to drop this cleanup if reviewers prefer a narrower PR.
   
   ## Are these changes tested?
   
   Behavior is unchanged for the built-in `FileScanConfig` path, so existing 
scan tests cover it. `cargo check`/`clippy`/`fmt` and the core tests pass 
locally.
   
   ## Are there any user-facing changes?
   
   Yes, a **breaking change** to the public `FileSource` trait. Custom 
`FileSource` implementors must update. A simple migration guide is incliuded.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to