[GitHub] [arrow] lidavidm commented on a diff in pull request #37690: GH-37626: [Java][Cpp] support using existing java file system

["lidavidm (via GitHub)" <gi...@apache.org>]

lidavidm commented on code in PR #37690:
URL: https://github.com/apache/arrow/pull/37690#discussion_r1327583560


##########
cpp/src/arrow/dataset/discovery.h:
##########
@@ -192,6 +192,10 @@ struct FileSystemFactoryOptions {
       ".",
       "_",
   };
+
+  /// when java context have a file system reference to be used
+  // then use this file system reference in the context
+  std::shared_ptr<void> file_system_java = nullptr;

Review Comment:
   I don't think this needs to be Java-specific at all.
   
   This should be `shared_ptr<FileSystem>`.



##########
cpp/src/arrow/dataset/discovery.cc:
##########
@@ -217,7 +217,7 @@ Result<std::shared_ptr<DatasetFactory>> FileSystemDatasetFactory::Make(
   // ARROW-12481.
   std::string internal_path;
   ARROW_ASSIGN_OR_RAISE(std::shared_ptr<fs::FileSystem> filesystem,
-                        fs::FileSystemFromUri(uri, &internal_path))
+                        fs::FileSystemFromUriAndFs(uri, &internal_path, options.file_system_java))

Review Comment:
   I think what you would do is, if a filesystem is provided, call FileSystem::PathFromUri to set `internal_path`. If it succeeds, then proceed, else raise an error.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org