address comments

apache · Aug 8, 2024 · 747a576 · 747a576
1 parent 603af2d
commit 747a576
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 20 deletions.
diff --git a/...ends-velox/src/main/java/org/apache/gluten/utils/VeloxFileSystemValidationJniWrapper.java b/...ends-velox/src/main/java/org/apache/gluten/utils/VeloxFileSystemValidationJniWrapper.java
@@ -18,5 +18,5 @@
 
 public class VeloxFileSystemValidationJniWrapper {
 
-  public static native boolean supportedPaths(String[] paths);
+  public static native boolean isSupportedByRegisteredFileSystems(String path);
 }
diff --git a/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxBackend.scala b/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxBackend.scala
@@ -76,11 +76,12 @@ object VeloxBackendSettings extends BackendSettingsApi {
       partTable: Boolean,
       rootPaths: Seq[String],
       paths: Seq[String]): ValidationResult = {
-    if (
-      !rootPaths.isEmpty && !VeloxFileSystemValidationJniWrapper.supportedPaths(rootPaths.toArray)
-    ) {
-      return ValidationResult.failed(
-        s"Schema of [$rootPaths] is not supported by registered file systems.")
+    // Skip native validation for local path, as local file system is always registered.
+    if (!rootPaths.isEmpty && !rootPaths.head.startsWith("file://")) {
+      if (!VeloxFileSystemValidationJniWrapper.isSupportedByRegisteredFileSystems(rootPaths.head)) {
+        return ValidationResult.failed(
+          s"Schema of [${rootPaths.head}] is not supported by registered file systems.")
+      }
     }
     // Validate if all types are supported.
     def validateTypes(validatorFunc: PartialFunction[StructField, String]): ValidationResult = {

diff --git a/backends-velox/src/test/scala/org/apache/gluten/execution/FallbackSuite.scala b/backends-velox/src/test/scala/org/apache/gluten/execution/FallbackSuite.scala
@@ -283,12 +283,17 @@ class FallbackSuite extends VeloxWholeStageTransformerSuite with AdaptiveSparkPl
               val rootPaths = fileScan(0).getRootPathsInternal
               assert(rootPaths.length == 1)
               assert(rootPaths(0).startsWith("file:/"))
-              assert(VeloxFileSystemValidationJniWrapper.supportedPaths(rootPaths.toArray))
+              assert(
+                VeloxFileSystemValidationJniWrapper.isSupportedByRegisteredFileSystems(
+                  rootPaths.head))
           }
         }
     }
 
-    assert(VeloxFileSystemValidationJniWrapper.supportedPaths(Array("file://test_path/")))
-    assert(!VeloxFileSystemValidationJniWrapper.supportedPaths(Array("unsupported://test_path")))
+    assert(
+      VeloxFileSystemValidationJniWrapper.isSupportedByRegisteredFileSystems("file:/test_path/"))
+    assert(
+      !VeloxFileSystemValidationJniWrapper.isSupportedByRegisteredFileSystems(
+        "unsupported://test_path"))
   }
 }
diff --git a/cpp/velox/jni/VeloxJniWrapper.cc b/cpp/velox/jni/VeloxJniWrapper.cc
@@ -260,19 +260,15 @@ JNIEXPORT jlong JNICALL Java_org_apache_gluten_utils_VeloxBatchAppenderJniWrappe
   JNI_METHOD_END(gluten::kInvalidObjectHandle)
 }
 
-JNIEXPORT bool JNICALL Java_org_apache_gluten_utils_VeloxFileSystemValidationJniWrapper_supportedPaths( // NOLINT
+JNIEXPORT jboolean JNICALL
+Java_org_apache_gluten_utils_VeloxFileSystemValidationJniWrapper_isSupportedByRegisteredFileSystems( // NOLINT
     JNIEnv* env,
     jclass,
-    jobjectArray stringArray) {
-  int size = env->GetArrayLength(stringArray);
-  for (int i = 0; i < size; i++) {
-    jstring string = (jstring)(env->GetObjectArrayElement(stringArray, i));
-    std::string path = jStringToCString(env, string);
-    if (!velox::filesystems::isPathSupportedByRegisteredFileSystems(path)) {
-      return false;
-    }
-  }
-  return true;
+    jstring path) {
+  JNI_METHOD_START
+  std::string filPath = jStringToCString(env, path);
+  return velox::filesystems::isPathSupportedByRegisteredFileSystems(filPath);
+  JNI_METHOD_END(false)
 }
 
 #ifdef __cplusplus