morristai opened a new issue, #26633: URL: https://github.com/apache/datafusion/issues/26633
### Is your feature request related to a problem or challenge? A plan rewrite that inspects a Parquet scan cannot tell which reader factory the scan uses. [`ParquetSource::parquet_file_reader_factory`](https://github.com/apache/datafusion/blob/f43f67b844189af217e252f9894fe66d02546124/datafusion/datasource-parquet/src/source.rs#L403) returns `&Arc<dyn ParquetFileReaderFactory>`, and the [trait](https://github.com/apache/datafusion/blob/f43f67b844189af217e252f9894fe66d02546124/datafusion/datasource-parquet/src/reader.rs#L71) is `Debug + Send + Sync + 'static` with no way to reach the concrete type. Our case: [Algeon](https://github.com/algeon-dev/algeon) replaces DataFusion Parquet scans with its own GPU reader when that reader can reproduce what the scan reads. A scan with `DefaultParquetFileReaderFactory` or `CachedParquetFileReaderFactory` qualifies. A caller-provided factory does not, because it may change bytes, metadata, caching or credentials. Today we tell them apart by checking that `format!("{factory:?}")` starts with `DefaultParquetFileReaderFactory { store: ` or `CachedParquetFileReaderFactory { store: `. That breaks if the derived `Debug` output changes, and a caller type with the same name and first field passes. ### Describe the solution you'd like Give `ParquetFileReaderFactory` an `Any` supertrait and the same inherent methods [`dyn FileSource`](https://github.com/apache/datafusion/blob/f43f67b844189af217e252f9894fe66d02546124/datafusion/datasource/src/file.rs#L414-L424) has: ```rust pub trait ParquetFileReaderFactory: Any + Debug + Send + Sync { ... } impl dyn ParquetFileReaderFactory { pub fn is<T: ParquetFileReaderFactory>(&self) -> bool { (self as &dyn Any).is::<T>() } pub fn downcast_ref<T: ParquetFileReaderFactory>(&self) -> Option<&T> { (self as &dyn Any).downcast_ref() } } ``` The trait already requires `'static`, so every existing implementation is `Any` and keeps compiling. `Any` implies `'static`, so the explicit bound can go, as in the other traits with an `Any` supertrait. This is the pattern #21572 describes and #21576 applied to `FileSource`, `DataSource`, `FileFormat` and `DataSink`. Those traits had an `as_any` to replace; this one never had any downcast path, so it was not part of that series. `ParquetFileSchemaProvider` in the same file has the same shape, but `ParquetSource` has no public accessor for it, so nothing outside the crate could downcast it. I would leave it alone. ### Describe alternatives you've considered - Keep matching on the `Debug` output, as described above. - Add a method such as `fn is_default(&self) -> bool` to the trait. That covers our check but not other uses, and the `Any` pattern is already the convention in these crates. ### Additional context None. ### Willingness to contribute None -- 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]
