From b270651157e61bd4eced9e066837ec95213654d1 Mon Sep 17 00:00:00 2001 From: Arun Sharma Date: Thu, 30 Jul 2026 10:42:24 -0700 Subject: [PATCH 1/2] fix: guard against non-Value params in Connection.execute + auto-convert boxed Java types Reject non-Value parameters in Connection.execute instead of crashing the JVM. Option A: Accept boxed Java values (String, Long, UUID, etc.) by reusing the type-conversion ladder from lbugValueCreateValue in bindJavaParamsToPreparedStatement. Option B: Guard unrecognized types with IllegalArgumentException instead of reinterpret_cast crash. - Moves the JNI type-conversion ladder into a reusable javaObjectToValue helper - Adds throwIllegalArgumentException and getClassName JNI helpers - Widens Connection.execute signature from Map to Map - Adds early-return guard in lbugConnectionExecute after param binding - Adds 7 new tests: raw String/Long/Double/Boolean, mixed raw+Value, unsupported type throws, Value-wrapped regression Fixes: #11 --- src/jni/lbug_java.cpp | 144 +++++++++++++++++- src/main/java/com/lbugdb/Connection.java | 21 ++- .../com/lbugdb/PreparedStatementTest.java | 114 +++++++++++++- 3 files changed, 271 insertions(+), 8 deletions(-) diff --git a/src/jni/lbug_java.cpp b/src/jni/lbug_java.cpp index 0a200e6..b9e188e 100644 --- a/src/jni/lbug_java.cpp +++ b/src/jni/lbug_java.cpp @@ -160,6 +160,14 @@ static void throwJNIException(JNIEnv* env, const char* message) { env->ThrowNew(exClass, message); } +static void throwIllegalArgumentException(JNIEnv* env, const char* message) { + jclass exClass = env->FindClass("java/lang/IllegalArgumentException"); + if (exClass == nullptr) { + return; + } + env->ThrowNew(exClass, message); +} + template static jobject callObjectMethodChecked(JNIEnv* env, jobject object, jmethodID method, Args... args) { @@ -504,6 +512,111 @@ std::string dataTypeToString(lbug_data_type_id dataType) { } } +// Convert a Java boxed primitive/String/UUID/etc. to an lbug_value*. +// Returns nullptr if the type is not recognised. +static lbug_value* javaObjectToValue(JNIEnv* env, jobject val) { + if (env->IsInstanceOf(val, J_C_Boolean)) { + jboolean value = callBooleanMethodChecked(env, val, J_C_Boolean_M_booleanValue); + return lbug_value_create_bool(static_cast(value)); + } else if (env->IsInstanceOf(val, J_C_Byte)) { + jbyte value = callByteMethodChecked(env, val, J_C_Byte_M_byteValue); + return lbug_value_create_int8(static_cast(value)); + } else if (env->IsInstanceOf(val, J_C_Short)) { + jshort value = callShortMethodChecked(env, val, J_C_Short_M_shortValue); + return lbug_value_create_int16(static_cast(value)); + } else if (env->IsInstanceOf(val, J_C_Integer)) { + jint value = callIntMethodChecked(env, val, J_C_Integer_M_intValue); + return lbug_value_create_int32(static_cast(value)); + } else if (env->IsInstanceOf(val, J_C_Long)) { + jlong value = callLongMethodChecked(env, val, J_C_Long_M_longValue); + return lbug_value_create_int64(static_cast(value)); + } else if (env->IsInstanceOf(val, J_C_BigInteger)) { + int64_t lower = + static_cast(callLongMethodChecked(env, val, J_C_BigInteger_M_longValue)); + jobject shifted = callObjectMethodChecked(env, val, J_C_BigInteger_M_shiftRight, 64); + int64_t upper = static_cast( + callLongMethodChecked(env, shifted, J_C_BigInteger_M_longValue)); + return lbug_value_create_int128({.low = static_cast(lower), .high = upper}); + } else if (env->IsInstanceOf(val, J_C_Float)) { + jfloat value = callFloatMethodChecked(env, val, J_C_Float_M_floatValue); + return lbug_value_create_float(static_cast(value)); + } else if (env->IsInstanceOf(val, J_C_Double)) { + jdouble value = callDoubleMethodChecked(env, val, J_C_Double_M_doubleValue); + return lbug_value_create_double(static_cast(value)); + } else if (env->IsInstanceOf(val, J_C_BigDecimal)) { + jobject normalized = + callObjectMethodChecked(env, val, J_C_BigDecimal_M_stripTrailingZeros); + jstring value = static_cast( + callObjectMethodChecked(env, normalized, J_C_BigDecimal_M_toString)); + std::string str = jstringToUtf8String(env, value); + auto precision = static_cast( + callIntMethodChecked(env, normalized, J_C_BigDecimal_M_precision)); + auto scale = + static_cast(callIntMethodChecked(env, normalized, J_C_BigDecimal_M_scale)); + if (precision > JAVA_DECIMAL_PRECISION_LIMIT) { + throw NotImplementedException( + std::format("Decimal precision cannot be greater than {}" + "Note: positive exponents contribute to precision", + JAVA_DECIMAL_PRECISION_LIMIT)); + } + return lbug_value_create_decimal( + str.c_str(), static_cast(precision), static_cast(scale)); + } else if (env->IsInstanceOf(val, J_C_String)) { + jstring value = static_cast(val); + std::string str = jstringToUtf8String(env, value); + return lbug_value_create_string(str.c_str()); + } else if (env->IsInstanceOf(val, J_C_InternalID)) { + return lbug_value_create_internal_id(getInternalID(env, val)); + } else if (env->IsInstanceOf(val, J_C_UUID)) { + jstring uuid = + static_cast(callObjectMethodChecked(env, val, J_C_UUID_M_toString)); + std::string uuidString = jstringToUtf8String(env, uuid); + return lbug_value_create_uuid(uuidString.c_str()); + } else if (env->IsInstanceOf(val, J_C_LocalDate)) { + int64_t days = + static_cast(callLongMethodChecked(env, val, J_C_LocalDate_M_toEpochDay)); + return lbug_value_create_date({.days = static_cast(days)}); + } else if (env->IsInstanceOf(val, J_C_Instant)) { + // TODO: Need to review this for overflow + int64_t seconds = static_cast( + callLongMethodChecked(env, val, J_C_LocalDate_M_getEpochSecond)); + int64_t nano = + static_cast(callLongMethodChecked(env, val, J_C_LocalDate_M_getNano)); + int64_t micro = (seconds * 1000000L) + (nano / 1000L); + return lbug_value_create_timestamp({.value = micro}); + } else if (env->IsInstanceOf(val, J_C_Duration)) { + auto milis = callLongMethodChecked(env, val, J_C_Duration_M_toMillis); + return lbug_value_create_interval({.months = 0, .days = 0, .micros = milis * 1000L}); + } + return nullptr; +} + +// Get the fully-qualified class name of a Java object as a std::string. +// Returns "" on error. +static std::string getClassName(JNIEnv* env, jobject obj) { + jclass clazz = env->GetObjectClass(obj); + if (clazz == nullptr) return ""; + jclass classClass = env->FindClass("java/lang/Class"); + if (classClass == nullptr) { + env->DeleteLocalRef(clazz); + return ""; + } + jmethodID getNameMethod = + env->GetMethodID(classClass, "getName", "()Ljava/lang/String;"); + if (getNameMethod == nullptr) { + env->DeleteLocalRef(classClass); + env->DeleteLocalRef(clazz); + return ""; + } + jstring classNameJStr = + static_cast(env->CallObjectMethod(clazz, getNameMethod)); + std::string result = jstringToUtf8String(env, classNameJStr); + env->DeleteLocalRef(classNameJStr); + env->DeleteLocalRef(classClass); + env->DeleteLocalRef(clazz); + return result; +} + void bindJavaParamsToPreparedStatement(JNIEnv* env, lbug_prepared_statement* preparedStatement, jobject javaMap) { jobject set = callObjectMethodChecked(env, javaMap, J_C_Map_M_entrySet); @@ -513,7 +626,33 @@ void bindJavaParamsToPreparedStatement(JNIEnv* env, lbug_prepared_statement* pre jstring key = (jstring)callObjectMethodChecked(env, entry, J_C_Map$Entry_M_getKey); jobject value = callObjectMethodChecked(env, entry, J_C_Map$Entry_M_getValue); std::string keyStr = jstringToUtf8String(env, key); - auto* clonedValue = lbug_value_clone(getValue(env, value)); + + lbug_value* clonedValue = nullptr; + + if (env->IsInstanceOf(value, J_C_Value)) { + // Already a Value — use the direct JNI path (Option A, existing behaviour) + clonedValue = lbug_value_clone(getValue(env, value)); + } else { + // Try conversion from a boxed Java type via the same ladder as + // Java_com_ladybugdb_Native_lbugValueCreateValue (Option A) + clonedValue = javaObjectToValue(env, value); + if (clonedValue == nullptr) { + // Not a recognised type — throw IllegalArgumentException instead + // of crashing (Option B) + std::string typeName = getClassName(env, value); + env->DeleteLocalRef(entry); + env->DeleteLocalRef(key); + env->DeleteLocalRef(value); + throwIllegalArgumentException(env, + ("Parameter '" + keyStr + "' has unsupported type " + typeName + + ". Accepted types: Value, Boolean, Byte, Short, Integer, " + "Long, BigInteger, Float, Double, BigDecimal, String, " + "InternalID, UUID, LocalDate, Instant, Duration") + .c_str()); + return; + } + } + auto state = lbug_prepared_statement_bind_value(preparedStatement, keyStr.c_str(), clonedValue); lbug_value_destroy(clonedValue); @@ -769,6 +908,9 @@ JNIEXPORT jobject JNICALL Java_com_ladybugdb_Native_lbugConnectionExecute(JNIEnv auto* conn = getConnection(env, thisConn); auto* ps = getPreparedStatement(env, preStm); bindJavaParamsToPreparedStatement(env, ps, paramMap); + if (env->ExceptionCheck()) { + return jobject(); + } auto* queryResult = new lbug_query_result(); lbug_state state = lbug_connection_execute(conn, ps, queryResult); if (state != LbugSuccess && queryResult->_query_result == nullptr) { diff --git a/src/main/java/com/lbugdb/Connection.java b/src/main/java/com/lbugdb/Connection.java index caeb6fa..ece4f3e 100644 --- a/src/main/java/com/lbugdb/Connection.java +++ b/src/main/java/com/lbugdb/Connection.java @@ -107,15 +107,24 @@ public PreparedStatement prepare(String queryStr) { /** * Executes the given prepared statement with args and returns the result. * - * @param ps: The prepared statement to execute. - * @param m: The parameter pack where each arg is a std::pair with the first element being parameter name and second - * element being parameter value + *

Values that are not already {@link Value} instances are automatically converted from + * their boxed Java type (e.g. {@link String}, {@link Long}, {@link java.util.UUID}, etc.). + * If a value's type is not supported, an {@link IllegalArgumentException} is thrown instead + * of crashing the JVM. + * + * @param ps The prepared statement to execute. + * @param params The parameter map. Each value must be a {@link Value} or one of the + * supported boxed types: Boolean, Byte, Short, Integer, Long, BigInteger, + * Float, Double, BigDecimal, String, InternalID, UUID, LocalDate, Instant, + * Duration. * @return The result of the query. - * @throws RuntimeException If the connection has been destroyed. + * @throws RuntimeException If the connection has been destroyed. + * @throws IllegalArgumentException If a parameter value has an unsupported type. */ - public QueryResult execute(PreparedStatement ps, Map m) { + @SuppressWarnings("unchecked") + public QueryResult execute(PreparedStatement ps, Map params) { checkNotDestroyed(); - return Native.lbugConnectionExecute(this, ps, m); + return Native.lbugConnectionExecute(this, ps, (Map) (Map) params); } /** diff --git a/src/test/java/com/lbugdb/PreparedStatementTest.java b/src/test/java/com/lbugdb/PreparedStatementTest.java index 3c5f0a1..690ad16 100644 --- a/src/test/java/com/lbugdb/PreparedStatementTest.java +++ b/src/test/java/com/lbugdb/PreparedStatementTest.java @@ -4,7 +4,8 @@ import static org.junit.jupiter.api.Assertions.*; -import java.util.Map; +import java.util.*; +import java.util.stream.Collectors; public class PreparedStatementTest extends TestBase { @@ -61,4 +62,115 @@ void PrepStmtSpecialString() { } } + @Test + void executeWithRawBoxedString() { + // Option A: raw String value auto-converted by C++ layer + String query = "MATCH (n:person) WHERE n.fName = $1 RETURN n.fName"; + try (PreparedStatement ps = conn.prepare(query)) { + Map raw = new HashMap<>(); + raw.put("1", "Alice"); + @SuppressWarnings("unchecked") + Map params = (Map) (Map) raw; + QueryResult result = conn.execute(ps, params); + assertTrue(result.isSuccess()); + assertTrue(result.hasNext()); + String got = result.getNext().getValue(0).getValue(); + assertEquals("Alice", got); + } + } + + @Test + void executeWithRawBoxedLong() { + // Option A: raw Long value auto-converted by C++ layer + String query = "MATCH (n:person) WHERE n.ID = $1 RETURN n.fName"; + try (PreparedStatement ps = conn.prepare(query)) { + Map raw = new HashMap<>(); + raw.put("1", 0L); + @SuppressWarnings("unchecked") + Map params = (Map) (Map) raw; + QueryResult result = conn.execute(ps, params); + assertTrue(result.isSuccess()); + assertTrue(result.hasNext()); + String got = result.getNext().getValue(0).getValue(); + assertEquals("Alice", got); + } + } + + @Test + void executeWithRawBoxedDouble() { + // Option A: raw Double value auto-converted + String query = "MATCH (n:person) WHERE n.eyeSight = $1 RETURN n.fName"; + try (PreparedStatement ps = conn.prepare(query)) { + Map raw = new HashMap<>(); + raw.put("1", 5.0); + @SuppressWarnings("unchecked") + Map params = (Map) (Map) raw; + QueryResult result = conn.execute(ps, params); + assertTrue(result.isSuccess()); + assertTrue(result.hasNext()); + String got = result.getNext().getValue(0).getValue(); + assertEquals("Alice", got); + } + } + + @Test + void executeWithRawBoxedBoolean() { + // Option A: raw Boolean value auto-converted + String query = "MATCH (n:person) WHERE n.isStudent = $1 RETURN n.fName"; + try (PreparedStatement ps = conn.prepare(query)) { + Map raw = new HashMap<>(); + raw.put("1", true); + @SuppressWarnings("unchecked") + Map params = (Map) (Map) raw; + QueryResult result = conn.execute(ps, params); + assertTrue(result.isSuccess()); + assertTrue(result.hasNext()); + } + } + + @Test + void executeWithUnsupportedTypeThrows() { + // Option B: unsupported type throws IllegalArgumentException instead of crashing + String query = "MATCH (n:person) WHERE n.fName = $1 RETURN n.fName"; + try (PreparedStatement ps = conn.prepare(query)) { + Map raw = new HashMap<>(); + raw.put("1", new ArrayList<>(List.of("not", "supported"))); + @SuppressWarnings("unchecked") + Map params = (Map) (Map) raw; + assertThrows(IllegalArgumentException.class, () -> { + conn.execute(ps, params); + }); + } + } + + @Test + void executeWithValueWrappedParamsRegression() { + // Regression: Value-wrapped path is unchanged + String query = "MATCH (n:person) WHERE n.fName = $1 RETURN n.fName"; + try (PreparedStatement ps = conn.prepare(query)) { + Map params = Map.of("1", new Value("Alice")); + QueryResult result = conn.execute(ps, params); + assertTrue(result.isSuccess()); + assertTrue(result.hasNext()); + String got = result.getNext().getValue(0).getValue(); + assertEquals("Alice", got); + } + } + + @Test + void executeWithMixedRawAndValueParams() { + // Option A: mixed raw and Value-wrapped params + String query = "MATCH (n:person) WHERE n.fName = $1 AND n.age = $2 RETURN n.fName"; + try (PreparedStatement ps = conn.prepare(query)) { + Map raw = new HashMap<>(); + raw.put("1", new Value("Alice")); + raw.put("2", 35L); + @SuppressWarnings("unchecked") + Map params = (Map) (Map) raw; + QueryResult result = conn.execute(ps, params); + assertTrue(result.isSuccess()); + assertTrue(result.hasNext()); + } + } + } From c0ea430110fdfb81a67433e56e80a596a28cdcf7 Mon Sep 17 00:00:00 2001 From: Arun Sharma Date: Thu, 30 Jul 2026 10:56:46 -0700 Subject: [PATCH 2/2] refactor: coerce params Java-side, drop JNI ladder, bump to Java 21 Move the boxed-Java-type-to-Value coercion from the JNI binding into Connection.execute on the Java side. The new coerceParams helper builds a LinkedHashMap from the user-supplied Map, preserving order and dispatching each entry through a single pattern-matching switch (JEP 441, GA in Java 21). Already-wrapped Value instances pass through; boxed types are auto-converted via Value(T); unsupported types and null surface as IllegalArgumentException with clear messages. This lets the native binding stay strictly typed (no @SuppressWarnings("unchecked") or raw-type cast in Connection.execute) and removes ~135 lines of dead code from lbug_java.cpp: - javaObjectToValue (the conversion ladder) - getClassName (only used to format the dead-branch error) - throwIllegalArgumentException (only called from the dead branch) - the else branch in bindJavaParamsToPreparedStatement bindJavaParamsToPreparedStatement collapses to a direct lbug_value_clone with a defensive IsInstanceOf check as a contract guard against any future API bypass. Bumps Java compile target from 1.8/11 to 21 in CMakeLists.txt, build.gradle, and the published POM. Adds kotlinOptions.jvmTarget = 21 so the Kotlin plugin does not refuse the build with "inconsistent JVM target" between compileTestJava and compileTestKotlin. CI matrix is already on 21 (java-workflow.yml, build-and-deploy.yml) so no CI churn. Downstream consumers will need Java 21+ at runtime. Adds two tests: - executeWithNullParamThrows: locks in the key-naming error - executeWithValueCreateNullParam: verifies the happy SQL-NULL path --- CMakeLists.txt | 2 +- build.gradle | 19 ++- src/jni/lbug_java.cpp | 155 ++---------------- src/main/java/com/lbugdb/Connection.java | 71 +++++++- .../com/lbugdb/PreparedStatementTest.java | 36 ++++ 5 files changed, 138 insertions(+), 145 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 76fdb70..14b770c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -71,7 +71,7 @@ foreach(artifact_spec IN LISTS LBUG_MAVEN_ARTIFACTS) list(APPEND LBUG_MAVEN_JARS "${jar_path}") endforeach() -set(CMAKE_JAVA_COMPILE_FLAGS -source 1.8 -target 1.8 -encoding utf-8) +set(CMAKE_JAVA_COMPILE_FLAGS -source 21 -target 21 -encoding utf-8) add_jar(lbug_java ${JAVA_SRC_FILES} INCLUDE_JARS ${LBUG_MAVEN_JARS} OUTPUT_DIR "${PROJECT_SOURCE_DIR}/build" diff --git a/build.gradle b/build.gradle index 2b6c49d..dbaa515 100644 --- a/build.gradle +++ b/build.gradle @@ -8,6 +8,19 @@ plugins { java { withSourcesJar() withJavadocJar() + // Pattern-matching switch in Connection.coerceParam requires Java 21+. + // Keep this in sync with CMAKE_JAVA_COMPILE_FLAGS in CMakeLists.txt and + // the java.version / maven.compiler.{source,target} POM properties below. + sourceCompatibility = JavaVersion.VERSION_21 + targetCompatibility = JavaVersion.VERSION_21 +} + +// Kotlin must target the same JVM level as Java, otherwise Gradle refuses +// to build (inconsistent jvm-target between compileTestJava and compileTestKotlin). +tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile).configureEach { + kotlinOptions { + jvmTarget = '21' + } } group = 'com.ladybugdb' @@ -63,9 +76,9 @@ publishing { properties = [ 'project.build.sourceEncoding' : 'UTF-8', 'project.reporting.outputEncoding': 'UTF-8', - 'java.version' : '11', - 'maven.compiler.source' : '11', - 'maven.compiler.target' : '11' + 'java.version' : '21', + 'maven.compiler.source' : '21', + 'maven.compiler.target' : '21' ] licenses { license { diff --git a/src/jni/lbug_java.cpp b/src/jni/lbug_java.cpp index b9e188e..bd7f9e0 100644 --- a/src/jni/lbug_java.cpp +++ b/src/jni/lbug_java.cpp @@ -160,14 +160,6 @@ static void throwJNIException(JNIEnv* env, const char* message) { env->ThrowNew(exClass, message); } -static void throwIllegalArgumentException(JNIEnv* env, const char* message) { - jclass exClass = env->FindClass("java/lang/IllegalArgumentException"); - if (exClass == nullptr) { - return; - } - env->ThrowNew(exClass, message); -} - template static jobject callObjectMethodChecked(JNIEnv* env, jobject object, jmethodID method, Args... args) { @@ -512,111 +504,6 @@ std::string dataTypeToString(lbug_data_type_id dataType) { } } -// Convert a Java boxed primitive/String/UUID/etc. to an lbug_value*. -// Returns nullptr if the type is not recognised. -static lbug_value* javaObjectToValue(JNIEnv* env, jobject val) { - if (env->IsInstanceOf(val, J_C_Boolean)) { - jboolean value = callBooleanMethodChecked(env, val, J_C_Boolean_M_booleanValue); - return lbug_value_create_bool(static_cast(value)); - } else if (env->IsInstanceOf(val, J_C_Byte)) { - jbyte value = callByteMethodChecked(env, val, J_C_Byte_M_byteValue); - return lbug_value_create_int8(static_cast(value)); - } else if (env->IsInstanceOf(val, J_C_Short)) { - jshort value = callShortMethodChecked(env, val, J_C_Short_M_shortValue); - return lbug_value_create_int16(static_cast(value)); - } else if (env->IsInstanceOf(val, J_C_Integer)) { - jint value = callIntMethodChecked(env, val, J_C_Integer_M_intValue); - return lbug_value_create_int32(static_cast(value)); - } else if (env->IsInstanceOf(val, J_C_Long)) { - jlong value = callLongMethodChecked(env, val, J_C_Long_M_longValue); - return lbug_value_create_int64(static_cast(value)); - } else if (env->IsInstanceOf(val, J_C_BigInteger)) { - int64_t lower = - static_cast(callLongMethodChecked(env, val, J_C_BigInteger_M_longValue)); - jobject shifted = callObjectMethodChecked(env, val, J_C_BigInteger_M_shiftRight, 64); - int64_t upper = static_cast( - callLongMethodChecked(env, shifted, J_C_BigInteger_M_longValue)); - return lbug_value_create_int128({.low = static_cast(lower), .high = upper}); - } else if (env->IsInstanceOf(val, J_C_Float)) { - jfloat value = callFloatMethodChecked(env, val, J_C_Float_M_floatValue); - return lbug_value_create_float(static_cast(value)); - } else if (env->IsInstanceOf(val, J_C_Double)) { - jdouble value = callDoubleMethodChecked(env, val, J_C_Double_M_doubleValue); - return lbug_value_create_double(static_cast(value)); - } else if (env->IsInstanceOf(val, J_C_BigDecimal)) { - jobject normalized = - callObjectMethodChecked(env, val, J_C_BigDecimal_M_stripTrailingZeros); - jstring value = static_cast( - callObjectMethodChecked(env, normalized, J_C_BigDecimal_M_toString)); - std::string str = jstringToUtf8String(env, value); - auto precision = static_cast( - callIntMethodChecked(env, normalized, J_C_BigDecimal_M_precision)); - auto scale = - static_cast(callIntMethodChecked(env, normalized, J_C_BigDecimal_M_scale)); - if (precision > JAVA_DECIMAL_PRECISION_LIMIT) { - throw NotImplementedException( - std::format("Decimal precision cannot be greater than {}" - "Note: positive exponents contribute to precision", - JAVA_DECIMAL_PRECISION_LIMIT)); - } - return lbug_value_create_decimal( - str.c_str(), static_cast(precision), static_cast(scale)); - } else if (env->IsInstanceOf(val, J_C_String)) { - jstring value = static_cast(val); - std::string str = jstringToUtf8String(env, value); - return lbug_value_create_string(str.c_str()); - } else if (env->IsInstanceOf(val, J_C_InternalID)) { - return lbug_value_create_internal_id(getInternalID(env, val)); - } else if (env->IsInstanceOf(val, J_C_UUID)) { - jstring uuid = - static_cast(callObjectMethodChecked(env, val, J_C_UUID_M_toString)); - std::string uuidString = jstringToUtf8String(env, uuid); - return lbug_value_create_uuid(uuidString.c_str()); - } else if (env->IsInstanceOf(val, J_C_LocalDate)) { - int64_t days = - static_cast(callLongMethodChecked(env, val, J_C_LocalDate_M_toEpochDay)); - return lbug_value_create_date({.days = static_cast(days)}); - } else if (env->IsInstanceOf(val, J_C_Instant)) { - // TODO: Need to review this for overflow - int64_t seconds = static_cast( - callLongMethodChecked(env, val, J_C_LocalDate_M_getEpochSecond)); - int64_t nano = - static_cast(callLongMethodChecked(env, val, J_C_LocalDate_M_getNano)); - int64_t micro = (seconds * 1000000L) + (nano / 1000L); - return lbug_value_create_timestamp({.value = micro}); - } else if (env->IsInstanceOf(val, J_C_Duration)) { - auto milis = callLongMethodChecked(env, val, J_C_Duration_M_toMillis); - return lbug_value_create_interval({.months = 0, .days = 0, .micros = milis * 1000L}); - } - return nullptr; -} - -// Get the fully-qualified class name of a Java object as a std::string. -// Returns "" on error. -static std::string getClassName(JNIEnv* env, jobject obj) { - jclass clazz = env->GetObjectClass(obj); - if (clazz == nullptr) return ""; - jclass classClass = env->FindClass("java/lang/Class"); - if (classClass == nullptr) { - env->DeleteLocalRef(clazz); - return ""; - } - jmethodID getNameMethod = - env->GetMethodID(classClass, "getName", "()Ljava/lang/String;"); - if (getNameMethod == nullptr) { - env->DeleteLocalRef(classClass); - env->DeleteLocalRef(clazz); - return ""; - } - jstring classNameJStr = - static_cast(env->CallObjectMethod(clazz, getNameMethod)); - std::string result = jstringToUtf8String(env, classNameJStr); - env->DeleteLocalRef(classNameJStr); - env->DeleteLocalRef(classClass); - env->DeleteLocalRef(clazz); - return result; -} - void bindJavaParamsToPreparedStatement(JNIEnv* env, lbug_prepared_statement* preparedStatement, jobject javaMap) { jobject set = callObjectMethodChecked(env, javaMap, J_C_Map_M_entrySet); @@ -627,31 +514,23 @@ void bindJavaParamsToPreparedStatement(JNIEnv* env, lbug_prepared_statement* pre jobject value = callObjectMethodChecked(env, entry, J_C_Map$Entry_M_getValue); std::string keyStr = jstringToUtf8String(env, key); - lbug_value* clonedValue = nullptr; - - if (env->IsInstanceOf(value, J_C_Value)) { - // Already a Value — use the direct JNI path (Option A, existing behaviour) - clonedValue = lbug_value_clone(getValue(env, value)); - } else { - // Try conversion from a boxed Java type via the same ladder as - // Java_com_ladybugdb_Native_lbugValueCreateValue (Option A) - clonedValue = javaObjectToValue(env, value); - if (clonedValue == nullptr) { - // Not a recognised type — throw IllegalArgumentException instead - // of crashing (Option B) - std::string typeName = getClassName(env, value); - env->DeleteLocalRef(entry); - env->DeleteLocalRef(key); - env->DeleteLocalRef(value); - throwIllegalArgumentException(env, - ("Parameter '" + keyStr + "' has unsupported type " + typeName - + ". Accepted types: Value, Boolean, Byte, Short, Integer, " - "Long, BigInteger, Float, Double, BigDecimal, String, " - "InternalID, UUID, LocalDate, Instant, Duration") - .c_str()); - return; - } - } + // The Java side (Connection.coerceParams) guarantees that every entry + // is already a Value — boxed primitives are converted there before the + // JNI call. We keep the IsInstanceOf check as a cheap contract guard: + // if it ever fails, something bypassed the public API and we'd rather + // fail loud than reinterpret_cast into the void. + if (!env->IsInstanceOf(value, J_C_Value)) { + env->DeleteLocalRef(entry); + env->DeleteLocalRef(key); + env->DeleteLocalRef(value); + throwJNIException(env, + ("Parameter '" + keyStr + + "' is not a Value — Connection.execute must be used as the entry point") + .c_str()); + return; + } + + lbug_value* clonedValue = lbug_value_clone(getValue(env, value)); auto state = lbug_prepared_statement_bind_value(preparedStatement, keyStr.c_str(), clonedValue); diff --git a/src/main/java/com/lbugdb/Connection.java b/src/main/java/com/lbugdb/Connection.java index ece4f3e..b43feda 100644 --- a/src/main/java/com/lbugdb/Connection.java +++ b/src/main/java/com/lbugdb/Connection.java @@ -1,7 +1,14 @@ package com.ladybugdb; -import java.util.Map; +import java.math.BigDecimal; +import java.math.BigInteger; +import java.time.Duration; +import java.time.Instant; +import java.time.LocalDate; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; +import java.util.UUID; import org.apache.arrow.c.ArrowSchema; import org.apache.arrow.memory.BufferAllocator; @@ -121,10 +128,68 @@ public PreparedStatement prepare(String queryStr) { * @throws RuntimeException If the connection has been destroyed. * @throws IllegalArgumentException If a parameter value has an unsupported type. */ - @SuppressWarnings("unchecked") public QueryResult execute(PreparedStatement ps, Map params) { checkNotDestroyed(); - return Native.lbugConnectionExecute(this, ps, (Map) (Map) params); + return Native.lbugConnectionExecute(this, ps, coerceParams(params)); + } + + /** + * Convert the user-supplied {@code Map} into the strict + * {@code Map} shape the JNI binding expects. Already-wrapped + * {@link Value} instances are passed through (the JNI will clone them when + * binding); boxed Java types are auto-converted via {@link Value#Value}. + * + *

Doing the conversion on the Java side keeps the public API ergonomic + * ({@code Map} instead of forcing users into a raw-type cast) + * while letting the native binding stay strictly typed — no + * {@code @SuppressWarnings("unchecked")} required, and the conversion logic + * is testable without round-tripping through JNI. + */ + private static Map coerceParams(Map params) { + Map coerced = new LinkedHashMap<>(params.size()); + for (Map.Entry e : params.entrySet()) { + coerced.put(e.getKey(), coerceParam(e.getKey(), e.getValue())); + } + return coerced; + } + + private static Value coerceParam(String key, Object v) { + // Pattern-matching switch (JEP 441, GA in Java 21): the JIT lowers + // this to a single type-table jump, so dispatch is O(1) per entry + // regardless of how many supported types we add. Hot path for users + // who bind a `Map` of mostly-uniform types — e.g. 1000 + // `Long` parameters — and previously paid 15 instanceof checks + // per element. + return switch (v) { + case null -> + // The JNI path used to surface null as IllegalArgumentException + // ("unsupported type null"); preserve that contract. Users who + // want a SQL NULL must build one explicitly via + // {@link Value#createNull()}. + throw new IllegalArgumentException( + "Parameter '" + key + "' is null; use Value.createNull() to bind SQL NULL."); + case Value value -> value; + case Boolean box -> new Value(box); + case Byte box -> new Value(box); + case Short box -> new Value(box); + case Integer box -> new Value(box); + case Long box -> new Value(box); + case BigInteger box -> new Value(box); + case Float box -> new Value(box); + case Double box -> new Value(box); + case BigDecimal box -> new Value(box); + case String box -> new Value(box); + case InternalID box -> new Value(box); + case UUID box -> new Value(box); + case LocalDate box -> new Value(box); + case Instant box -> new Value(box); + case Duration box -> new Value(box); + default -> throw new IllegalArgumentException( + "Parameter '" + key + "' has unsupported type " + v.getClass().getName() + + ". Accepted types: Value, Boolean, Byte, Short, Integer, Long, " + + "BigInteger, Float, Double, BigDecimal, String, InternalID, UUID, " + + "LocalDate, Instant, Duration"); + }; } /** diff --git a/src/test/java/com/lbugdb/PreparedStatementTest.java b/src/test/java/com/lbugdb/PreparedStatementTest.java index 690ad16..5ca4529 100644 --- a/src/test/java/com/lbugdb/PreparedStatementTest.java +++ b/src/test/java/com/lbugdb/PreparedStatementTest.java @@ -173,4 +173,40 @@ void executeWithMixedRawAndValueParams() { } } + @Test + void executeWithNullParamThrows() { + // After the refactor, nulls are caught on the Java side with a clear + // error message naming the offending key, instead of crashing inside + // the JNI conversion ladder. + String query = "MATCH (n:person) WHERE n.fName = $1 RETURN n.fName"; + try (PreparedStatement ps = conn.prepare(query)) { + Map raw = new HashMap<>(); + raw.put("1", null); + @SuppressWarnings("unchecked") + Map params = (Map) (Map) raw; + IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> { + conn.execute(ps, params); + }); + assertTrue(ex.getMessage().contains("'1'"), + "exception should name the offending key, got: " + ex.getMessage()); + } + } + + @Test + void executeWithValueCreateNullParam() { + // Value.createNull() is the explicit way to bind a SQL NULL; it should + // round-trip cleanly through the new coercion path. + String query = "MATCH (n:person) WHERE n.fName = $1 RETURN n.fName"; + try (PreparedStatement ps = conn.prepare(query)) { + Map raw = new HashMap<>(); + raw.put("1", Value.createNull()); + @SuppressWarnings("unchecked") + Map params = (Map) (Map) raw; + QueryResult result = conn.execute(ps, params); + assertTrue(result.isSuccess()); + // SQL NULL never matches n.fName, so no rows. + assertFalse(result.hasNext()); + } + } + }