Merge '[bindings/java] Fix failing tests ' from Kim Seon Woo

## The purpose of this PR
- Do not create a new `io` and instead use `io` which is used when
creating `LimboDatabase`.
- Run the loop of `step` function on the rust side(better for
performance as we don't have to switch between java <-> rust)
## Changes
- Java
  - handle invalid cases
  - remove @Disabled annotation from tests
- Rust
  - add loops in the step function
## Reference
- https://github.com/tursodatabase/limbo/issues/615

Closes #761
This commit is contained in:
Pekka Enberg
2025-01-25 08:08:34 +02:00
7 changed files with 69 additions and 83 deletions

View File

@@ -9,16 +9,19 @@ use jni::sys::jlong;
use jni::JNIEnv;
use limbo_core::Connection;
use std::rc::Rc;
use std::sync::Arc;
#[derive(Clone)]
#[allow(dead_code)]
pub struct LimboConnection {
// Because java's LimboConnection is 1:1 mapped to limbo connection, we can use Rc
pub(crate) conn: Rc<Connection>,
pub(crate) io: Rc<dyn limbo_core::IO>,
// Because io is shared across multiple `LimboConnection`s, wrap it with Arc
pub(crate) io: Arc<dyn limbo_core::IO>,
}
impl LimboConnection {
pub fn new(conn: Rc<Connection>, io: Rc<dyn limbo_core::IO>) -> Self {
pub fn new(conn: Rc<Connection>, io: Arc<dyn limbo_core::IO>) -> Self {
LimboConnection { conn, io }
}
@@ -69,7 +72,7 @@ pub extern "system" fn Java_org_github_tursodatabase_core_LimboConnection_prepar
};
match connection.conn.prepare(sql) {
Ok(stmt) => LimboStatement::new(stmt).to_ptr(),
Ok(stmt) => LimboStatement::new(stmt, connection.clone()).to_ptr(),
Err(e) => {
set_err_msg_and_throw_exception(
&mut env,

View File

@@ -5,16 +5,16 @@ use jni::objects::{JByteArray, JObject};
use jni::sys::{jint, jlong};
use jni::JNIEnv;
use limbo_core::Database;
use std::rc::Rc;
use std::sync::Arc;
struct LimboDB {
db: Arc<Database>,
io: Arc<dyn limbo_core::IO>,
}
impl LimboDB {
pub fn new(db: Arc<Database>) -> Self {
LimboDB { db }
pub fn new(db: Arc<Database>, io: Arc<dyn limbo_core::IO>) -> Self {
LimboDB { db, io }
}
pub fn to_ptr(self) -> jlong {
@@ -76,14 +76,13 @@ pub extern "system" fn Java_org_github_tursodatabase_core_LimboDB_openUtf8<'loca
}
};
LimboDB::new(db).to_ptr()
LimboDB::new(db, io).to_ptr()
}
#[no_mangle]
pub extern "system" fn Java_org_github_tursodatabase_core_LimboDB_connect0<'local>(
mut env: JNIEnv<'local>,
obj: JObject<'local>,
file_path_byte_arr: JByteArray<'local>,
db_pointer: jlong,
) -> jlong {
let db = match to_limbo_db(db_pointer) {
@@ -94,41 +93,7 @@ pub extern "system" fn Java_org_github_tursodatabase_core_LimboDB_connect0<'loca
}
};
let path = match env
.convert_byte_array(file_path_byte_arr)
.map_err(|e| e.to_string())
{
Ok(bytes) => match String::from_utf8(bytes) {
Ok(s) => s,
Err(e) => {
set_err_msg_and_throw_exception(&mut env, obj, LIMBO_ETC, e.to_string());
return 0;
}
},
Err(e) => {
set_err_msg_and_throw_exception(&mut env, obj, LIMBO_ETC, e.to_string());
return 0;
}
};
let io: Rc<dyn limbo_core::IO> = match path.as_str() {
":memory:" => match limbo_core::MemoryIO::new() {
Ok(io) => Rc::new(io),
Err(e) => {
set_err_msg_and_throw_exception(&mut env, obj, LIMBO_ETC, e.to_string());
return 0;
}
},
_ => match limbo_core::PlatformIO::new() {
Ok(io) => Rc::new(io),
Err(e) => {
set_err_msg_and_throw_exception(&mut env, obj, LIMBO_ETC, e.to_string());
return 0;
}
},
};
let conn = LimboConnection::new(db.db.connect(), io);
let conn = LimboConnection::new(db.db.connect(), db.io.clone());
conn.to_ptr()
}

View File

@@ -1,5 +1,6 @@
use crate::errors::Result;
use crate::errors::{LimboError, LIMBO_ETC};
use crate::limbo_connection::LimboConnection;
use crate::utils::set_err_msg_and_throw_exception;
use jni::objects::{JObject, JValue};
use jni::sys::jlong;
@@ -7,6 +8,7 @@ use jni::JNIEnv;
use limbo_core::{Statement, StepResult};
pub const STEP_RESULT_ID_ROW: i32 = 10;
#[allow(dead_code)]
pub const STEP_RESULT_ID_IO: i32 = 20;
pub const STEP_RESULT_ID_DONE: i32 = 30;
pub const STEP_RESULT_ID_INTERRUPT: i32 = 40;
@@ -15,11 +17,12 @@ pub const STEP_RESULT_ID_ERROR: i32 = 60;
pub struct LimboStatement {
pub(crate) stmt: Statement,
pub(crate) connection: LimboConnection,
}
impl LimboStatement {
pub fn new(stmt: Statement) -> Self {
LimboStatement { stmt }
pub fn new(stmt: Statement, connection: LimboConnection) -> Self {
LimboStatement { stmt, connection }
}
pub fn to_ptr(self) -> jlong {
@@ -50,30 +53,38 @@ pub extern "system" fn Java_org_github_tursodatabase_core_LimboStatement_step<'l
Ok(stmt) => stmt,
Err(e) => {
set_err_msg_and_throw_exception(&mut env, obj, LIMBO_ETC, e.to_string());
return JObject::null();
return to_limbo_step_result(&mut env, STEP_RESULT_ID_ERROR, None);
}
};
match stmt.stmt.step() {
Ok(StepResult::Row(row)) => match row_to_obj_array(&mut env, &row) {
Ok(row) => to_limbo_step_result(&mut env, STEP_RESULT_ID_ROW, Some(row)),
Err(e) => {
set_err_msg_and_throw_exception(&mut env, obj, LIMBO_ETC, e.to_string());
to_limbo_step_result(&mut env, STEP_RESULT_ID_ERROR, None)
loop {
let step_result = match stmt.stmt.step() {
Ok(result) => result,
Err(_) => return to_limbo_step_result(&mut env, STEP_RESULT_ID_ERROR, None),
};
match step_result {
StepResult::Row(row) => {
return match row_to_obj_array(&mut env, &row) {
Ok(row) => to_limbo_step_result(&mut env, STEP_RESULT_ID_ROW, Some(row)),
Err(e) => {
set_err_msg_and_throw_exception(&mut env, obj, LIMBO_ETC, e.to_string());
to_limbo_step_result(&mut env, STEP_RESULT_ID_ERROR, None)
}
}
}
},
Ok(StepResult::IO) => match env.new_object_array(0, "java/lang/Object", JObject::null()) {
Ok(row) => to_limbo_step_result(&mut env, STEP_RESULT_ID_IO, Some(row.into())),
Err(e) => {
set_err_msg_and_throw_exception(&mut env, obj, LIMBO_ETC, e.to_string());
to_limbo_step_result(&mut env, STEP_RESULT_ID_ERROR, None)
StepResult::IO => {
if let Err(e) = stmt.connection.io.run_once() {
set_err_msg_and_throw_exception(&mut env, obj, LIMBO_ETC, e.to_string());
return to_limbo_step_result(&mut env, STEP_RESULT_ID_ERROR, None);
}
}
},
Ok(StepResult::Done) => to_limbo_step_result(&mut env, STEP_RESULT_ID_DONE, None),
Ok(StepResult::Interrupt) => to_limbo_step_result(&mut env, STEP_RESULT_ID_INTERRUPT, None),
Ok(StepResult::Busy) => to_limbo_step_result(&mut env, STEP_RESULT_ID_BUSY, None),
_ => to_limbo_step_result(&mut env, STEP_RESULT_ID_ERROR, None),
StepResult::Done => return to_limbo_step_result(&mut env, STEP_RESULT_ID_DONE, None),
StepResult::Interrupt => {
return to_limbo_step_result(&mut env, STEP_RESULT_ID_INTERRUPT, None)
}
StepResult::Busy => return to_limbo_step_result(&mut env, STEP_RESULT_ID_BUSY, None),
}
}
}

View File

@@ -1,5 +1,9 @@
package org.github.tursodatabase.core;
import static org.github.tursodatabase.utils.ByteArrayUtils.stringToUtf8ByteArray;
import java.sql.SQLException;
import java.util.concurrent.locks.ReentrantLock;
import org.github.tursodatabase.LimboErrorCode;
import org.github.tursodatabase.annotations.NativeInvocation;
@@ -8,12 +12,6 @@ import org.github.tursodatabase.utils.LimboExceptionUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.sql.SQLException;
import java.sql.SQLFeatureNotSupportedException;
import java.util.concurrent.locks.ReentrantLock;
import static org.github.tursodatabase.utils.ByteArrayUtils.stringToUtf8ByteArray;
/**
* This class provides a thin JNI layer over the SQLite3 C API.
*/
@@ -39,7 +37,7 @@ public final class LimboDB extends AbstractDB {
* Loads the SQLite interface backend.
*/
public static void load() {
if (isLoaded) return;
if (isLoaded) {return;}
try {
System.loadLibrary("_limbo_java");
@@ -49,7 +47,7 @@ public final class LimboDB extends AbstractDB {
}
/**
* @param url e.g. "jdbc:sqlite:fileName
* @param url e.g. "jdbc:sqlite:fileName
* @param filePath e.g. path to file
*/
public static LimboDB create(String url, String filePath) throws SQLException {
@@ -86,7 +84,9 @@ public final class LimboDB extends AbstractDB {
byte[] filePathBytes = stringToUtf8ByteArray(filePath);
if (filePathBytes == null) {
throw LimboExceptionUtils.buildLimboException(LimboErrorCode.LIMBO_ETC.code, "File path cannot be converted to byteArray. File name: " + filePath);
throw LimboExceptionUtils.buildLimboException(
LimboErrorCode.LIMBO_ETC.code,
"File path cannot be converted to byteArray. File name: " + filePath);
}
dbPointer = openUtf8(filePathBytes, openFlags);
@@ -95,14 +95,10 @@ public final class LimboDB extends AbstractDB {
@Override
public long connect() throws SQLException {
byte[] filePathBytes = stringToUtf8ByteArray(filePath);
if (filePathBytes == null) {
throw LimboExceptionUtils.buildLimboException(LimboErrorCode.LIMBO_ETC.code, "File path cannot be converted to byteArray. File name: " + filePath);
}
return connect0(filePathBytes, dbPointer);
return connect0(dbPointer);
}
private native long connect0(byte[] path, long databasePtr) throws SQLException;
private native long connect0(long databasePtr) throws SQLException;
@VisibleForTesting
native void throwJavaException(int errorCode) throws SQLException;
@@ -110,7 +106,7 @@ public final class LimboDB extends AbstractDB {
/**
* Throws formatted SQLException with error code and message.
*
* @param errorCode Error code.
* @param errorCode Error code.
* @param errorMessageBytes Error message.
*/
@NativeInvocation(invokedFrom = "limbo_db.rs")

View File

@@ -64,6 +64,11 @@ public class LimboResultSet {
row++;
}
if (lastStepResult.isInInvalidState()) {
open = false;
throw new SQLException("step() returned invalid result: " + lastStepResult);
}
pastLastRow = lastStepResult.isDone();
if (pastLastRow) {
open = false;

View File

@@ -13,6 +13,7 @@ public class LimboStepResult {
private static final int STEP_RESULT_ID_IO = 20;
private static final int STEP_RESULT_ID_DONE = 30;
private static final int STEP_RESULT_ID_INTERRUPT = 40;
// Indicates that the database file could not be written because of concurrent activity by some other connection
private static final int STEP_RESULT_ID_BUSY = 50;
private static final int STEP_RESULT_ID_ERROR = 60;
@@ -41,6 +42,14 @@ public class LimboStepResult {
return stepResultId == STEP_RESULT_ID_DONE;
}
public boolean isInInvalidState() {
// current implementation doesn't allow STEP_RESULT_ID_IO to be returned
return stepResultId == STEP_RESULT_ID_IO ||
stepResultId == STEP_RESULT_ID_INTERRUPT ||
stepResultId == STEP_RESULT_ID_BUSY ||
stepResultId == STEP_RESULT_ID_ERROR;
}
@Override
public String toString() {
return "LimboStepResult{" +

View File

@@ -9,7 +9,6 @@ import java.util.Properties;
import org.github.tursodatabase.TestUtils;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
class JDBC4ResultSetTest {
@@ -27,7 +26,6 @@ class JDBC4ResultSetTest {
}
@Test
@Disabled("https://github.com/tursodatabase/limbo/pull/743#issuecomment-2600746904")
void invoking_next_before_the_last_row_should_return_true() throws Exception {
stmt.executeUpdate("CREATE TABLE users (id INT PRIMARY KEY, username TEXT);");
stmt.executeUpdate("INSERT INTO users VALUES (1, 'sinwoo');");
@@ -41,7 +39,6 @@ class JDBC4ResultSetTest {
}
@Test
@Disabled("https://github.com/tursodatabase/limbo/pull/743#issuecomment-2600746904")
void invoking_next_after_the_last_row_should_return_false() throws Exception {
stmt.executeUpdate("CREATE TABLE users (id INT PRIMARY KEY, username TEXT);");
stmt.executeUpdate("INSERT INTO users VALUES (1, 'sinwoo');");