Skip to content

Commit 0111e0a

Browse files
committed
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<String, Value> from the user-supplied Map<String, ?>, 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
1 parent 7eee51e commit 0111e0a

5 files changed

Lines changed: 138 additions & 145 deletions

File tree

‎CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ foreach(artifact_spec IN LISTS LBUG_MAVEN_ARTIFACTS)
7171
list(APPEND LBUG_MAVEN_JARS "${jar_path}")
7272
endforeach()
7373

74-
set(CMAKE_JAVA_COMPILE_FLAGS -source 1.8 -target 1.8 -encoding utf-8)
74+
set(CMAKE_JAVA_COMPILE_FLAGS -source 21 -target 21 -encoding utf-8)
7575
add_jar(lbug_java ${JAVA_SRC_FILES}
7676
INCLUDE_JARS ${LBUG_MAVEN_JARS}
7777
OUTPUT_DIR "${PROJECT_SOURCE_DIR}/build"

‎build.gradle‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,19 @@ plugins {
88
java {
99
withSourcesJar()
1010
withJavadocJar()
11+
// Pattern-matching switch in Connection.coerceParam requires Java 21+.
12+
// Keep this in sync with CMAKE_JAVA_COMPILE_FLAGS in CMakeLists.txt and
13+
// the java.version / maven.compiler.{source,target} POM properties below.
14+
sourceCompatibility = JavaVersion.VERSION_21
15+
targetCompatibility = JavaVersion.VERSION_21
16+
}
17+
18+
// Kotlin must target the same JVM level as Java, otherwise Gradle refuses
19+
// to build (inconsistent jvm-target between compileTestJava and compileTestKotlin).
20+
tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile).configureEach {
21+
kotlinOptions {
22+
jvmTarget = '21'
23+
}
1124
}
1225

1326
group = 'com.ladybugdb'
@@ -63,9 +76,9 @@ publishing {
6376
properties = [
6477
'project.build.sourceEncoding' : 'UTF-8',
6578
'project.reporting.outputEncoding': 'UTF-8',
66-
'java.version' : '11',
67-
'maven.compiler.source' : '11',
68-
'maven.compiler.target' : '11'
79+
'java.version' : '21',
80+
'maven.compiler.source' : '21',
81+
'maven.compiler.target' : '21'
6982
]
7083
licenses {
7184
license {

‎src/jni/lbug_java.cpp‎

Lines changed: 17 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -160,14 +160,6 @@ static void throwJNIException(JNIEnv* env, const char* message) {
160160
env->ThrowNew(exClass, message);
161161
}
162162

163-
static void throwIllegalArgumentException(JNIEnv* env, const char* message) {
164-
jclass exClass = env->FindClass("java/lang/IllegalArgumentException");
165-
if (exClass == nullptr) {
166-
return;
167-
}
168-
env->ThrowNew(exClass, message);
169-
}
170-
171163
template<typename... Args>
172164
static jobject callObjectMethodChecked(JNIEnv* env, jobject object, jmethodID method,
173165
Args... args) {
@@ -512,111 +504,6 @@ std::string dataTypeToString(lbug_data_type_id dataType) {
512504
}
513505
}
514506

515-
// Convert a Java boxed primitive/String/UUID/etc. to an lbug_value*.
516-
// Returns nullptr if the type is not recognised.
517-
static lbug_value* javaObjectToValue(JNIEnv* env, jobject val) {
518-
if (env->IsInstanceOf(val, J_C_Boolean)) {
519-
jboolean value = callBooleanMethodChecked(env, val, J_C_Boolean_M_booleanValue);
520-
return lbug_value_create_bool(static_cast<bool>(value));
521-
} else if (env->IsInstanceOf(val, J_C_Byte)) {
522-
jbyte value = callByteMethodChecked(env, val, J_C_Byte_M_byteValue);
523-
return lbug_value_create_int8(static_cast<int8_t>(value));
524-
} else if (env->IsInstanceOf(val, J_C_Short)) {
525-
jshort value = callShortMethodChecked(env, val, J_C_Short_M_shortValue);
526-
return lbug_value_create_int16(static_cast<int16_t>(value));
527-
} else if (env->IsInstanceOf(val, J_C_Integer)) {
528-
jint value = callIntMethodChecked(env, val, J_C_Integer_M_intValue);
529-
return lbug_value_create_int32(static_cast<int32_t>(value));
530-
} else if (env->IsInstanceOf(val, J_C_Long)) {
531-
jlong value = callLongMethodChecked(env, val, J_C_Long_M_longValue);
532-
return lbug_value_create_int64(static_cast<int64_t>(value));
533-
} else if (env->IsInstanceOf(val, J_C_BigInteger)) {
534-
int64_t lower =
535-
static_cast<int64_t>(callLongMethodChecked(env, val, J_C_BigInteger_M_longValue));
536-
jobject shifted = callObjectMethodChecked(env, val, J_C_BigInteger_M_shiftRight, 64);
537-
int64_t upper = static_cast<int64_t>(
538-
callLongMethodChecked(env, shifted, J_C_BigInteger_M_longValue));
539-
return lbug_value_create_int128({.low = static_cast<uint64_t>(lower), .high = upper});
540-
} else if (env->IsInstanceOf(val, J_C_Float)) {
541-
jfloat value = callFloatMethodChecked(env, val, J_C_Float_M_floatValue);
542-
return lbug_value_create_float(static_cast<float>(value));
543-
} else if (env->IsInstanceOf(val, J_C_Double)) {
544-
jdouble value = callDoubleMethodChecked(env, val, J_C_Double_M_doubleValue);
545-
return lbug_value_create_double(static_cast<double>(value));
546-
} else if (env->IsInstanceOf(val, J_C_BigDecimal)) {
547-
jobject normalized =
548-
callObjectMethodChecked(env, val, J_C_BigDecimal_M_stripTrailingZeros);
549-
jstring value = static_cast<jstring>(
550-
callObjectMethodChecked(env, normalized, J_C_BigDecimal_M_toString));
551-
std::string str = jstringToUtf8String(env, value);
552-
auto precision = static_cast<int32_t>(
553-
callIntMethodChecked(env, normalized, J_C_BigDecimal_M_precision));
554-
auto scale =
555-
static_cast<int32_t>(callIntMethodChecked(env, normalized, J_C_BigDecimal_M_scale));
556-
if (precision > JAVA_DECIMAL_PRECISION_LIMIT) {
557-
throw NotImplementedException(
558-
std::format("Decimal precision cannot be greater than {}"
559-
"Note: positive exponents contribute to precision",
560-
JAVA_DECIMAL_PRECISION_LIMIT));
561-
}
562-
return lbug_value_create_decimal(
563-
str.c_str(), static_cast<uint32_t>(precision), static_cast<uint32_t>(scale));
564-
} else if (env->IsInstanceOf(val, J_C_String)) {
565-
jstring value = static_cast<jstring>(val);
566-
std::string str = jstringToUtf8String(env, value);
567-
return lbug_value_create_string(str.c_str());
568-
} else if (env->IsInstanceOf(val, J_C_InternalID)) {
569-
return lbug_value_create_internal_id(getInternalID(env, val));
570-
} else if (env->IsInstanceOf(val, J_C_UUID)) {
571-
jstring uuid =
572-
static_cast<jstring>(callObjectMethodChecked(env, val, J_C_UUID_M_toString));
573-
std::string uuidString = jstringToUtf8String(env, uuid);
574-
return lbug_value_create_uuid(uuidString.c_str());
575-
} else if (env->IsInstanceOf(val, J_C_LocalDate)) {
576-
int64_t days =
577-
static_cast<int64_t>(callLongMethodChecked(env, val, J_C_LocalDate_M_toEpochDay));
578-
return lbug_value_create_date({.days = static_cast<int32_t>(days)});
579-
} else if (env->IsInstanceOf(val, J_C_Instant)) {
580-
// TODO: Need to review this for overflow
581-
int64_t seconds = static_cast<int64_t>(
582-
callLongMethodChecked(env, val, J_C_LocalDate_M_getEpochSecond));
583-
int64_t nano =
584-
static_cast<int64_t>(callLongMethodChecked(env, val, J_C_LocalDate_M_getNano));
585-
int64_t micro = (seconds * 1000000L) + (nano / 1000L);
586-
return lbug_value_create_timestamp({.value = micro});
587-
} else if (env->IsInstanceOf(val, J_C_Duration)) {
588-
auto milis = callLongMethodChecked(env, val, J_C_Duration_M_toMillis);
589-
return lbug_value_create_interval({.months = 0, .days = 0, .micros = milis * 1000L});
590-
}
591-
return nullptr;
592-
}
593-
594-
// Get the fully-qualified class name of a Java object as a std::string.
595-
// Returns "<unknown>" on error.
596-
static std::string getClassName(JNIEnv* env, jobject obj) {
597-
jclass clazz = env->GetObjectClass(obj);
598-
if (clazz == nullptr) return "<unknown>";
599-
jclass classClass = env->FindClass("java/lang/Class");
600-
if (classClass == nullptr) {
601-
env->DeleteLocalRef(clazz);
602-
return "<unknown>";
603-
}
604-
jmethodID getNameMethod =
605-
env->GetMethodID(classClass, "getName", "()Ljava/lang/String;");
606-
if (getNameMethod == nullptr) {
607-
env->DeleteLocalRef(classClass);
608-
env->DeleteLocalRef(clazz);
609-
return "<unknown>";
610-
}
611-
jstring classNameJStr =
612-
static_cast<jstring>(env->CallObjectMethod(clazz, getNameMethod));
613-
std::string result = jstringToUtf8String(env, classNameJStr);
614-
env->DeleteLocalRef(classNameJStr);
615-
env->DeleteLocalRef(classClass);
616-
env->DeleteLocalRef(clazz);
617-
return result;
618-
}
619-
620507
void bindJavaParamsToPreparedStatement(JNIEnv* env, lbug_prepared_statement* preparedStatement,
621508
jobject javaMap) {
622509
jobject set = callObjectMethodChecked(env, javaMap, J_C_Map_M_entrySet);
@@ -627,31 +514,23 @@ void bindJavaParamsToPreparedStatement(JNIEnv* env, lbug_prepared_statement* pre
627514
jobject value = callObjectMethodChecked(env, entry, J_C_Map$Entry_M_getValue);
628515
std::string keyStr = jstringToUtf8String(env, key);
629516

630-
lbug_value* clonedValue = nullptr;
631-
632-
if (env->IsInstanceOf(value, J_C_Value)) {
633-
// Already a Value — use the direct JNI path (Option A, existing behaviour)
634-
clonedValue = lbug_value_clone(getValue(env, value));
635-
} else {
636-
// Try conversion from a boxed Java type via the same ladder as
637-
// Java_com_ladybugdb_Native_lbugValueCreateValue (Option A)
638-
clonedValue = javaObjectToValue(env, value);
639-
if (clonedValue == nullptr) {
640-
// Not a recognised type — throw IllegalArgumentException instead
641-
// of crashing (Option B)
642-
std::string typeName = getClassName(env, value);
643-
env->DeleteLocalRef(entry);
644-
env->DeleteLocalRef(key);
645-
env->DeleteLocalRef(value);
646-
throwIllegalArgumentException(env,
647-
("Parameter '" + keyStr + "' has unsupported type " + typeName
648-
+ ". Accepted types: Value, Boolean, Byte, Short, Integer, "
649-
"Long, BigInteger, Float, Double, BigDecimal, String, "
650-
"InternalID, UUID, LocalDate, Instant, Duration")
651-
.c_str());
652-
return;
653-
}
654-
}
517+
// The Java side (Connection.coerceParams) guarantees that every entry
518+
// is already a Value — boxed primitives are converted there before the
519+
// JNI call. We keep the IsInstanceOf check as a cheap contract guard:
520+
// if it ever fails, something bypassed the public API and we'd rather
521+
// fail loud than reinterpret_cast into the void.
522+
if (!env->IsInstanceOf(value, J_C_Value)) {
523+
env->DeleteLocalRef(entry);
524+
env->DeleteLocalRef(key);
525+
env->DeleteLocalRef(value);
526+
throwJNIException(env,
527+
("Parameter '" + keyStr
528+
+ "' is not a Value — Connection.execute must be used as the entry point")
529+
.c_str());
530+
return;
531+
}
532+
533+
lbug_value* clonedValue = lbug_value_clone(getValue(env, value));
655534

656535
auto state =
657536
lbug_prepared_statement_bind_value(preparedStatement, keyStr.c_str(), clonedValue);

‎src/main/java/com/lbugdb/Connection.java‎

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
package com.ladybugdb;
22

3-
import java.util.Map;
3+
import java.math.BigDecimal;
4+
import java.math.BigInteger;
5+
import java.time.Duration;
6+
import java.time.Instant;
7+
import java.time.LocalDate;
8+
import java.util.LinkedHashMap;
49
import java.util.List;
10+
import java.util.Map;
11+
import java.util.UUID;
512

613
import org.apache.arrow.c.ArrowSchema;
714
import org.apache.arrow.memory.BufferAllocator;
@@ -121,10 +128,68 @@ public PreparedStatement prepare(String queryStr) {
121128
* @throws RuntimeException If the connection has been destroyed.
122129
* @throws IllegalArgumentException If a parameter value has an unsupported type.
123130
*/
124-
@SuppressWarnings("unchecked")
125131
public QueryResult execute(PreparedStatement ps, Map<String, ?> params) {
126132
checkNotDestroyed();
127-
return Native.lbugConnectionExecute(this, ps, (Map<String, Value>) (Map<?, ?>) params);
133+
return Native.lbugConnectionExecute(this, ps, coerceParams(params));
134+
}
135+
136+
/**
137+
* Convert the user-supplied {@code Map<String, ?>} into the strict
138+
* {@code Map<String, Value>} shape the JNI binding expects. Already-wrapped
139+
* {@link Value} instances are passed through (the JNI will clone them when
140+
* binding); boxed Java types are auto-converted via {@link Value#Value}.
141+
*
142+
* <p>Doing the conversion on the Java side keeps the public API ergonomic
143+
* ({@code Map<String, ?>} instead of forcing users into a raw-type cast)
144+
* while letting the native binding stay strictly typed — no
145+
* {@code @SuppressWarnings("unchecked")} required, and the conversion logic
146+
* is testable without round-tripping through JNI.
147+
*/
148+
private static Map<String, Value> coerceParams(Map<String, ?> params) {
149+
Map<String, Value> coerced = new LinkedHashMap<>(params.size());
150+
for (Map.Entry<String, ?> e : params.entrySet()) {
151+
coerced.put(e.getKey(), coerceParam(e.getKey(), e.getValue()));
152+
}
153+
return coerced;
154+
}
155+
156+
private static Value coerceParam(String key, Object v) {
157+
// Pattern-matching switch (JEP 441, GA in Java 21): the JIT lowers
158+
// this to a single type-table jump, so dispatch is O(1) per entry
159+
// regardless of how many supported types we add. Hot path for users
160+
// who bind a `Map<String, ?>` of mostly-uniform types — e.g. 1000
161+
// `Long` parameters — and previously paid 15 instanceof checks
162+
// per element.
163+
return switch (v) {
164+
case null ->
165+
// The JNI path used to surface null as IllegalArgumentException
166+
// ("unsupported type null"); preserve that contract. Users who
167+
// want a SQL NULL must build one explicitly via
168+
// {@link Value#createNull()}.
169+
throw new IllegalArgumentException(
170+
"Parameter '" + key + "' is null; use Value.createNull() to bind SQL NULL.");
171+
case Value value -> value;
172+
case Boolean box -> new Value(box);
173+
case Byte box -> new Value(box);
174+
case Short box -> new Value(box);
175+
case Integer box -> new Value(box);
176+
case Long box -> new Value(box);
177+
case BigInteger box -> new Value(box);
178+
case Float box -> new Value(box);
179+
case Double box -> new Value(box);
180+
case BigDecimal box -> new Value(box);
181+
case String box -> new Value(box);
182+
case InternalID box -> new Value(box);
183+
case UUID box -> new Value(box);
184+
case LocalDate box -> new Value(box);
185+
case Instant box -> new Value(box);
186+
case Duration box -> new Value(box);
187+
default -> throw new IllegalArgumentException(
188+
"Parameter '" + key + "' has unsupported type " + v.getClass().getName()
189+
+ ". Accepted types: Value, Boolean, Byte, Short, Integer, Long, "
190+
+ "BigInteger, Float, Double, BigDecimal, String, InternalID, UUID, "
191+
+ "LocalDate, Instant, Duration");
192+
};
128193
}
129194

130195
/**

‎src/test/java/com/lbugdb/PreparedStatementTest.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,4 +173,40 @@ void executeWithMixedRawAndValueParams() {
173173
}
174174
}
175175

176+
@Test
177+
void executeWithNullParamThrows() {
178+
// After the refactor, nulls are caught on the Java side with a clear
179+
// error message naming the offending key, instead of crashing inside
180+
// the JNI conversion ladder.
181+
String query = "MATCH (n:person) WHERE n.fName = $1 RETURN n.fName";
182+
try (PreparedStatement ps = conn.prepare(query)) {
183+
Map<String, Object> raw = new HashMap<>();
184+
raw.put("1", null);
185+
@SuppressWarnings("unchecked")
186+
Map<String, ?> params = (Map<String, ?>) (Map<?, ?>) raw;
187+
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> {
188+
conn.execute(ps, params);
189+
});
190+
assertTrue(ex.getMessage().contains("'1'"),
191+
"exception should name the offending key, got: " + ex.getMessage());
192+
}
193+
}
194+
195+
@Test
196+
void executeWithValueCreateNullParam() {
197+
// Value.createNull() is the explicit way to bind a SQL NULL; it should
198+
// round-trip cleanly through the new coercion path.
199+
String query = "MATCH (n:person) WHERE n.fName = $1 RETURN n.fName";
200+
try (PreparedStatement ps = conn.prepare(query)) {
201+
Map<String, Object> raw = new HashMap<>();
202+
raw.put("1", Value.createNull());
203+
@SuppressWarnings("unchecked")
204+
Map<String, ?> params = (Map<String, ?>) (Map<?, ?>) raw;
205+
QueryResult result = conn.execute(ps, params);
206+
assertTrue(result.isSuccess());
207+
// SQL NULL never matches n.fName, so no rows.
208+
assertFalse(result.hasNext());
209+
}
210+
}
211+
176212
}

0 commit comments

Comments
 (0)