tustvold commented on code in PR #2572:
URL: https://github.com/apache/arrow-datafusion/pull/2572#discussion_r880621119


##########
datafusion/core/src/datasource/file_format/mod.rs:
##########
@@ -52,7 +52,11 @@ pub trait FileFormat: Send + Sync + fmt::Debug {
     /// be analysed up to a given number of records or files (as specified in 
the
     /// format config) then give the estimated common schema. This might fail 
if
     /// the files have schemas that cannot be merged.
-    async fn infer_schema(&self, readers: ObjectReaderStream) -> 
Result<SchemaRef>;
+    async fn infer_schema(
+        &self,
+        store: &dyn ObjectStore,

Review Comment:
   I've made this change, but it grates with me... There doesn't seem to be a 
good reason why an implementation would need an Arc reference, and it makes for 
lots of opaque constructions like
   
   ```
   let store = Arc::new(LocalFileSystem {}) as _;
   ```
   
   I dunno, `&Arc` feels like a bit of a code-smell, either you want a 
reference or you want an owned type, I dunno...



-- 
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]

Reply via email to