yahoNanJing commented on a change in pull request #1779:
URL: https://github.com/apache/arrow-datafusion/pull/1779#discussion_r803258324



##########
File path: datafusion/src/datasource/object_store/mod.rs
##########
@@ -219,25 +220,42 @@ impl ObjectStoreRegistry {
 
     /// Get a suitable store for the URI based on it's scheme. For example:
     /// - URI with scheme `file://` or no schema will return the default 
LocalFS store
-    /// - URI with scheme `s3://` will return the S3 store if it's registered
-    /// Returns a tuple with the store and the path of the file in that store
-    /// (URI=scheme://path).
+    /// - URI with scheme `s3://host:port` will return the S3 store if it's 
registered
+    /// Returns a tuple with the store and the self-described uri of the file 
in that store
     pub fn get_by_uri<'a>(
         &self,
         uri: &'a str,
     ) -> Result<(Arc<dyn ObjectStore>, &'a str)> {
-        if let Some((scheme, path)) = uri.split_once("://") {
-            let stores = self.object_stores.read();
-            let store = stores
-                .get(&*scheme.to_lowercase())
-                .map(Clone::clone)
-                .ok_or_else(|| {
-                    DataFusionError::Internal(format!(
-                        "No suitable object store found for {}",
-                        scheme
-                    ))
-                })?;
-            Ok((store, path))
+        // We do not support the remote object store on Windows OS

Review comment:
       Thanks @alamb. Finally I get your point. To use the object store, there 
are three hierarchies: ObjectStoreRegistry -> ObjectStore -> 
ObjectStoreInstance. The ObjectStore is just for managing one kind of object 
store rather than being the instance. Previously I misunderstood it as a real 
instance.
   
   One more question, for some object store, they support many kinds of 
schemes. For example, HDFS support:
   - file://
   - hdfs://
   - viewfs://
   
   I still think the returned path should include the original scheme info 
rather than throw it away. Then for the object store instance, it will be able 
to deal with the path for different kinds of schemes. What do you think?




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