some comments

apache · Mar 7, 2025 · 0e0b809 · 0e0b809
1 parent bcc27c1
commit 0e0b809
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 5 deletions.
diff --git a/java/driver/jni/src/main/cpp/jni_wrapper.cc b/java/driver/jni/src/main/cpp/jni_wrapper.cc
@@ -32,6 +32,8 @@
 
 namespace {
 
+/// Internal exception.  Meant to be used with RaiseAdbcException and
+///   CHECK_ADBC_ERROR.
 struct AdbcException {
   AdbcStatusCode code;
   std::string message;
@@ -88,6 +90,7 @@ struct AdbcException {
   }
 };
 
+/// Signal an error to Java.
 void RaiseAdbcException(AdbcStatusCode code, const AdbcError& error) {
   assert(code != ADBC_STATUS_OK);
   throw AdbcException{
@@ -96,6 +99,7 @@ void RaiseAdbcException(AdbcStatusCode code, const AdbcError& error) {
   };
 }
 
+/// Check the result of an ADBC call and raise an exception to Java if it failed.
 #define CHECK_ADBC_ERROR(expr, error)      \
   do {                                     \
     AdbcStatusCode status = (expr);        \
@@ -104,6 +108,7 @@ void RaiseAdbcException(AdbcStatusCode code, const AdbcError& error) {
     }                                      \
   } while (0)
 
+/// Require that a Java class exists or error.
 jclass RequireImplClass(JNIEnv* env, std::string_view name) {
   static std::string kPrefix = "org/apache/arrow/adbc/driver/jni/impl/";
   std::string full_name = kPrefix + std::string(name);
@@ -117,6 +122,7 @@ jclass RequireImplClass(JNIEnv* env, std::string_view name) {
   return klass;
 }
 
+/// Require that a Java method exists or error.
 jmethodID RequireMethod(JNIEnv* env, jclass klass, std::string_view name,
                         std::string_view signature) {
   jmethodID method = env->GetMethodID(klass, name.data(), signature.data());
@@ -141,11 +147,12 @@ struct JniStringView {
   explicit JniStringView(JNIEnv* env, jstring jni_string)
       : env(env), jni_string(jni_string), value(nullptr) {
     if (jni_string == nullptr) {
-      // TODO:
+      throw AdbcException{ADBC_STATUS_INTERNAL, "Java string was nullptr"};
     }
     value = env->GetStringUTFChars(jni_string, nullptr);
     if (value == nullptr) {
-      // TODO:
+      throw AdbcException{ADBC_STATUS_INTERNAL,
+                          "Java string was nullptr (could not get string contents)"};
     }
   }
 

diff --git a/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/JniLoader.java b/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/JniLoader.java
@@ -27,18 +27,21 @@
 import java.util.Map;
 import org.apache.arrow.adbc.core.AdbcException;
 
+/** Singleton wrapper protecting access to JNI functions. */
 public enum JniLoader {
   INSTANCE;
 
   JniLoader() {
+    // The JAR may contain multiple binaries for different platforms, so load the appropriate one.
     final String libraryName = "adbc_driver_jni";
     String libraryToLoad =
         libraryName + "/" + getNormalizedArch() + "/" + System.mapLibraryName(libraryName);
 
     try {
       InputStream is = JniLoader.class.getClassLoader().getResourceAsStream(libraryToLoad);
       if (is == null) {
-        throw new FileNotFoundException(libraryToLoad);
+        throw new FileNotFoundException(
+            "No JNI library for current platform, missing from JAR: " + libraryToLoad);
       }
       File temp =
           File.createTempFile("adbc-jni-", ".tmp", new File(System.getProperty("java.io.tmpdir")));
@@ -49,11 +52,12 @@ public enum JniLoader {
       }
       Runtime.getRuntime().load(temp.getAbsolutePath());
     } catch (IOException e) {
-      throw new IllegalStateException("Error loading native library: " + e);
+      throw new IllegalStateException("Error loading native library " + libraryToLoad, e);
     }
   }
 
   private String getNormalizedArch() {
+    // Be consistent with our CMake config
     String arch = System.getProperty("os.arch").toLowerCase(Locale.US);
     switch (arch) {
       case "amd64":

diff --git a/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeAdbc.java b/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeAdbc.java
@@ -19,7 +19,8 @@
 
 import org.apache.arrow.adbc.core.AdbcException;
 
-public class NativeAdbc {
+/** All the JNI methods. Don't use this directly, prefer {@link JniLoader}. */
+class NativeAdbc {
   static native NativeDatabaseHandle openDatabase(int version, String[] parameters)
       throws AdbcException;