alamb commented on code in PR #26920:
URL: https://github.com/apache/datafusion/pull/26920#discussion_r4427513530
##########
datafusion/datasource-parquet/src/reader.rs:
##########
@@ -77,6 +77,32 @@ pub trait ParquetFileReaderFactory: Debug + Send + Sync +
'static {
metadata_size_hint: Option<usize>,
metrics: &ExecutionPlanMetricsSet,
) -> datafusion_common::Result<Box<dyn AsyncFileReader + Send>>;
+
+ /// Like [`Self::create_reader`], but the reader should record into
+ /// `file_metrics`, the [`ParquetFileMetrics`] the caller has already
+ /// registered for this file, rather than registering another set in
+ /// `metrics`.
+ ///
+ /// The parquet opener calls this so that each file it opens registers one
+ /// set of file metrics. The default implementation ignores `file_metrics`
+ /// and calls [`Self::create_reader`], so existing implementations keep
+ /// their behavior.
+ fn create_reader_with_file_metrics(
Review Comment:
Should we deprecate `create_reader`
If we are going to change how the construction works, can we please make it
more future proof api
```rust
/// argument struct so we can just add new fields without changing the trait
signature again
struct CreateReaderArgs {
...
}
// Bonus points for leaving the fields non pub and adding builder functions
/ a buulder
// now trait method takes arguments
fn create(args: CreateReaderArgs) -> ...
```
--
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]
