From 4dd2d1c64ab4ea3a625f766024b6934263b1303c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=84=A0=EC=9A=B0?= Date: Mon, 27 Jan 2025 20:20:10 +0900 Subject: [PATCH 1/6] Implement close() on LimboStatement --- bindings/java/rs_src/limbo_statement.rs | 9 +++++++ .../tursodatabase/core/LimboResultSet.java | 11 ++++++++- .../tursodatabase/core/LimboStatement.java | 10 ++++++++ .../tursodatabase/jdbc4/JDBC4ResultSet.java | 5 ++-- .../jdbc4/JDBC4ResultSetTest.java | 24 +++++++++++++++++++ 5 files changed, 55 insertions(+), 4 deletions(-) diff --git a/bindings/java/rs_src/limbo_statement.rs b/bindings/java/rs_src/limbo_statement.rs index 7de4b2c19..f8a9706e2 100644 --- a/bindings/java/rs_src/limbo_statement.rs +++ b/bindings/java/rs_src/limbo_statement.rs @@ -88,6 +88,15 @@ pub extern "system" fn Java_org_github_tursodatabase_core_LimboStatement_step<'l } } +#[no_mangle] +pub extern "system" fn Java_org_github_tursodatabase_core_LimboStatement__1close<'local>( + _env: JNIEnv<'local>, + _obj: JObject<'local>, + stmt_ptr: jlong +) { + LimboStatement::drop(stmt_ptr); +} + fn row_to_obj_array<'local>( env: &mut JNIEnv<'local>, row: &limbo_core::Row, diff --git a/bindings/java/src/main/java/org/github/tursodatabase/core/LimboResultSet.java b/bindings/java/src/main/java/org/github/tursodatabase/core/LimboResultSet.java index c6cb8d00e..6633dbd33 100644 --- a/bindings/java/src/main/java/org/github/tursodatabase/core/LimboResultSet.java +++ b/bindings/java/src/main/java/org/github/tursodatabase/core/LimboResultSet.java @@ -50,7 +50,11 @@ public class LimboResultSet { * cursor can only move forward. */ public boolean next() throws SQLException { - if (!open || isEmptyResultSet || pastLastRow) { + if (!open) { + throw new SQLException("The resultSet is not open"); + } + + if (isEmptyResultSet || pastLastRow) { return false; // completed ResultSet } @@ -97,6 +101,11 @@ public class LimboResultSet { } } + public void close() throws SQLException { + this.statement.close(); + this.open = false; + } + @Override public String toString() { return "LimboResultSet{" diff --git a/bindings/java/src/main/java/org/github/tursodatabase/core/LimboStatement.java b/bindings/java/src/main/java/org/github/tursodatabase/core/LimboStatement.java index c749e27cc..df82b71e3 100644 --- a/bindings/java/src/main/java/org/github/tursodatabase/core/LimboStatement.java +++ b/bindings/java/src/main/java/org/github/tursodatabase/core/LimboStatement.java @@ -67,6 +67,16 @@ public class LimboStatement { LimboExceptionUtils.throwLimboException(errorCode, errorMessageBytes); } + /** + * Closes the current statement and releases any resources associated with it. This method calls + * the native `_close` method to perform the actual closing operation. + */ + public void close() { + _close(statementPointer); + } + + private native void _close(long statementPointer); + @Override public String toString() { return "LimboStatement{" diff --git a/bindings/java/src/main/java/org/github/tursodatabase/jdbc4/JDBC4ResultSet.java b/bindings/java/src/main/java/org/github/tursodatabase/jdbc4/JDBC4ResultSet.java index 867b2688e..092bf4d44 100644 --- a/bindings/java/src/main/java/org/github/tursodatabase/jdbc4/JDBC4ResultSet.java +++ b/bindings/java/src/main/java/org/github/tursodatabase/jdbc4/JDBC4ResultSet.java @@ -25,7 +25,7 @@ public class JDBC4ResultSet implements ResultSet { @Override public void close() throws SQLException { - // TODO + resultSet.close(); } @Override @@ -866,8 +866,7 @@ public class JDBC4ResultSet implements ResultSet { @Override public boolean isClosed() throws SQLException { - // TODO - return false; + return !resultSet.isOpen(); } @Override diff --git a/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java b/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java index f764a9361..20f002733 100644 --- a/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java +++ b/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java @@ -57,4 +57,28 @@ class JDBC4ResultSetTest { // as well assertFalse(resultSet.next()); } + + @Test + void resultSet_close_test() throws Exception { + stmt.executeUpdate("CREATE TABLE users (id INT PRIMARY KEY, username TEXT);"); + stmt.executeUpdate("INSERT INTO users VALUES (2, 'seonwoo');"); + stmt.executeQuery("SELECT * FROM users"); + ResultSet resultSet = stmt.getResultSet(); + + assertFalse(resultSet.isClosed()); + resultSet.close(); + assertTrue(resultSet.isClosed()); + } + + @Test + void calling_methods_on_closed_resultSet_should_throw_exception() throws Exception { + stmt.executeUpdate("CREATE TABLE users (id INT PRIMARY KEY, username TEXT);"); + stmt.executeUpdate("INSERT INTO users VALUES (2, 'seonwoo');"); + stmt.executeQuery("SELECT * FROM users"); + ResultSet resultSet = stmt.getResultSet(); + resultSet.close(); + assertTrue(resultSet.isClosed()); + + resultSet.next(); + } } From e48d7aa763dc995b920bbacb68c7902551b504a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=84=A0=EC=9A=B0?= Date: Mon, 27 Jan 2025 20:31:37 +0900 Subject: [PATCH 2/6] Add `consumeAll` method in LimboResultSet and let JDBC4Statement to use it --- .../tursodatabase/core/LimboResultSet.java | 16 +++++++++++++--- .../tursodatabase/jdbc4/JDBC4Statement.java | 4 +--- .../tursodatabase/jdbc4/JDBC4ResultSetTest.java | 4 +++- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/bindings/java/src/main/java/org/github/tursodatabase/core/LimboResultSet.java b/bindings/java/src/main/java/org/github/tursodatabase/core/LimboResultSet.java index 6633dbd33..598ef22c4 100644 --- a/bindings/java/src/main/java/org/github/tursodatabase/core/LimboResultSet.java +++ b/bindings/java/src/main/java/org/github/tursodatabase/core/LimboResultSet.java @@ -39,6 +39,19 @@ public class LimboResultSet { this.statement = statement; } + /** + * Consumes all the rows in this result set until the {@link #next()} method returns `false`. + * + * @throws SQLException if the result set is not open or if an error occurs while iterating. + */ + public void consumeAll() throws SQLException { + if (!open) { + throw new SQLException("The result set is not open"); + } + + while (next()) {} + } + /** * Moves the cursor forward one row from its current position. A {@link LimboResultSet} cursor is * initially positioned before the first fow; the first call to the method next makes @@ -74,9 +87,6 @@ public class LimboResultSet { } pastLastRow = lastStepResult.isDone(); - if (pastLastRow) { - open = false; - } return !pastLastRow; } diff --git a/bindings/java/src/main/java/org/github/tursodatabase/jdbc4/JDBC4Statement.java b/bindings/java/src/main/java/org/github/tursodatabase/jdbc4/JDBC4Statement.java index eee4c95a3..1005d2e27 100644 --- a/bindings/java/src/main/java/org/github/tursodatabase/jdbc4/JDBC4Statement.java +++ b/bindings/java/src/main/java/org/github/tursodatabase/jdbc4/JDBC4Statement.java @@ -65,9 +65,7 @@ public class JDBC4Statement implements Statement { requireNonNull(statement, "statement should not be null after running execute method"); final LimboResultSet resultSet = statement.getResultSet(); - while (resultSet.isOpen()) { - resultSet.next(); - } + resultSet.consumeAll(); // TODO: return update count; return 0; diff --git a/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java b/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java index 20f002733..7d62a245b 100644 --- a/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java +++ b/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java @@ -1,9 +1,11 @@ package org.github.tursodatabase.jdbc4; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.sql.ResultSet; +import java.sql.SQLException; import java.sql.Statement; import java.util.Properties; import org.github.tursodatabase.TestUtils; @@ -79,6 +81,6 @@ class JDBC4ResultSetTest { resultSet.close(); assertTrue(resultSet.isClosed()); - resultSet.next(); + assertThrows(SQLException.class, resultSet::next); } } From c18418bed00dc79fbbf33d66db20dc9fc97a68c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=84=A0=EC=9A=B0?= Date: Mon, 27 Jan 2025 20:35:11 +0900 Subject: [PATCH 3/6] Nit --- bindings/java/rs_src/limbo_statement.rs | 1 - .../java/org/github/tursodatabase/core/LimboResultSet.java | 4 +++- .../org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/bindings/java/rs_src/limbo_statement.rs b/bindings/java/rs_src/limbo_statement.rs index f8a9706e2..4e39e411a 100644 --- a/bindings/java/rs_src/limbo_statement.rs +++ b/bindings/java/rs_src/limbo_statement.rs @@ -29,7 +29,6 @@ impl LimboStatement { Box::into_raw(Box::new(self)) as jlong } - #[allow(dead_code)] pub fn drop(ptr: jlong) { let _boxed = unsafe { Box::from_raw(ptr as *mut LimboStatement) }; } diff --git a/bindings/java/src/main/java/org/github/tursodatabase/core/LimboResultSet.java b/bindings/java/src/main/java/org/github/tursodatabase/core/LimboResultSet.java index 598ef22c4..ec6e63a26 100644 --- a/bindings/java/src/main/java/org/github/tursodatabase/core/LimboResultSet.java +++ b/bindings/java/src/main/java/org/github/tursodatabase/core/LimboResultSet.java @@ -1,5 +1,6 @@ package org.github.tursodatabase.core; +import java.sql.ResultSet; import java.sql.SQLException; import org.github.tursodatabase.annotations.Nullable; import org.slf4j.Logger; @@ -40,7 +41,8 @@ public class LimboResultSet { } /** - * Consumes all the rows in this result set until the {@link #next()} method returns `false`. + * Consumes all the rows in this {@link ResultSet} until the {@link #next()} method returns + * `false`. * * @throws SQLException if the result set is not open or if an error occurs while iterating. */ diff --git a/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java b/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java index 7d62a245b..905b6f0f3 100644 --- a/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java +++ b/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java @@ -61,7 +61,7 @@ class JDBC4ResultSetTest { } @Test - void resultSet_close_test() throws Exception { + void close_resultSet_test() throws Exception { stmt.executeUpdate("CREATE TABLE users (id INT PRIMARY KEY, username TEXT);"); stmt.executeUpdate("INSERT INTO users VALUES (2, 'seonwoo');"); stmt.executeQuery("SELECT * FROM users"); From eeed305b076eadd5ec2e261908f5c4d7447536e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=84=A0=EC=9A=B0?= Date: Mon, 27 Jan 2025 20:40:43 +0900 Subject: [PATCH 4/6] Nit --- bindings/java/rs_src/limbo_statement.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindings/java/rs_src/limbo_statement.rs b/bindings/java/rs_src/limbo_statement.rs index 4e39e411a..aed8e7d99 100644 --- a/bindings/java/rs_src/limbo_statement.rs +++ b/bindings/java/rs_src/limbo_statement.rs @@ -91,7 +91,7 @@ pub extern "system" fn Java_org_github_tursodatabase_core_LimboStatement_step<'l pub extern "system" fn Java_org_github_tursodatabase_core_LimboStatement__1close<'local>( _env: JNIEnv<'local>, _obj: JObject<'local>, - stmt_ptr: jlong + stmt_ptr: jlong, ) { LimboStatement::drop(stmt_ptr); } From a82c459ed0d163208c2c3685f132e4e57b5b463a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=84=A0=EC=9A=B0?= Date: Tue, 28 Jan 2025 09:20:07 +0900 Subject: [PATCH 5/6] Implement close on LimboStatement and JDBC4Statement independently --- .../tursodatabase/core/LimboResultSet.java | 1 - .../tursodatabase/core/LimboStatement.java | 18 ++++++++++- .../tursodatabase/jdbc4/JDBC4Statement.java | 32 +++++++++++-------- .../core/LimboStatementTest.java | 31 ++++++++++++++++++ .../jdbc4/JDBC4StatementTest.java | 19 +++++++++++ 5 files changed, 85 insertions(+), 16 deletions(-) create mode 100644 bindings/java/src/test/java/org/github/tursodatabase/core/LimboStatementTest.java diff --git a/bindings/java/src/main/java/org/github/tursodatabase/core/LimboResultSet.java b/bindings/java/src/main/java/org/github/tursodatabase/core/LimboResultSet.java index ec6e63a26..b226c53d0 100644 --- a/bindings/java/src/main/java/org/github/tursodatabase/core/LimboResultSet.java +++ b/bindings/java/src/main/java/org/github/tursodatabase/core/LimboResultSet.java @@ -114,7 +114,6 @@ public class LimboResultSet { } public void close() throws SQLException { - this.statement.close(); this.open = false; } diff --git a/bindings/java/src/main/java/org/github/tursodatabase/core/LimboStatement.java b/bindings/java/src/main/java/org/github/tursodatabase/core/LimboStatement.java index df82b71e3..8566c403e 100644 --- a/bindings/java/src/main/java/org/github/tursodatabase/core/LimboStatement.java +++ b/bindings/java/src/main/java/org/github/tursodatabase/core/LimboStatement.java @@ -21,6 +21,8 @@ public class LimboStatement { private final long statementPointer; private final LimboResultSet resultSet; + private boolean closed; + // TODO: what if the statement we ran was DDL, update queries and etc. Should we still create a // resultSet? public LimboStatement(String sql, long statementPointer) { @@ -71,12 +73,26 @@ public class LimboStatement { * Closes the current statement and releases any resources associated with it. This method calls * the native `_close` method to perform the actual closing operation. */ - public void close() { + public void close() throws SQLException { + if (closed) { + return; + } + this.resultSet.close(); _close(statementPointer); + closed = true; } private native void _close(long statementPointer); + /** + * Checks if the statement is closed. + * + * @return true if the statement is closed, false otherwise. + */ + public boolean isClosed() { + return closed; + } + @Override public String toString() { return "LimboStatement{" diff --git a/bindings/java/src/main/java/org/github/tursodatabase/jdbc4/JDBC4Statement.java b/bindings/java/src/main/java/org/github/tursodatabase/jdbc4/JDBC4Statement.java index 1005d2e27..7cbf6f69d 100644 --- a/bindings/java/src/main/java/org/github/tursodatabase/jdbc4/JDBC4Statement.java +++ b/bindings/java/src/main/java/org/github/tursodatabase/jdbc4/JDBC4Statement.java @@ -19,6 +19,8 @@ public class JDBC4Statement implements Statement { private final LimboConnection connection; @Nullable private LimboStatement statement = null; + // Because JDBC4Statement has different life cycle in compared to LimboStatement, let's use this + // field to manage JDBC4Statement lifecycle private boolean closed; private boolean closeOnCompletion; @@ -73,8 +75,14 @@ public class JDBC4Statement implements Statement { @Override public void close() throws SQLException { - clearGeneratedKeys(); - internalClose(); + if (closed) { + return; + } + + if (this.statement != null) { + this.statement.close(); + } + closed = true; } @@ -148,8 +156,7 @@ public class JDBC4Statement implements Statement { */ @Override public boolean execute(String sql) throws SQLException { - internalClose(); - + ensureOpen(); return this.withConnectionTimeout( () -> { try { @@ -296,8 +303,7 @@ public class JDBC4Statement implements Statement { @Override public boolean isClosed() throws SQLException { - // TODO - return false; + return this.closed; } @Override @@ -344,14 +350,6 @@ public class JDBC4Statement implements Statement { return false; } - protected void internalClose() throws SQLException { - // TODO - } - - protected void clearGeneratedKeys() throws SQLException { - // TODO - } - protected void updateGeneratedKeys() throws SQLException { // TODO } @@ -376,4 +374,10 @@ public class JDBC4Statement implements Statement { protected interface SQLCallable { T call() throws SQLException; } + + private void ensureOpen() throws SQLException { + if (closed) { + throw new SQLException("Statement is closed"); + } + } } diff --git a/bindings/java/src/test/java/org/github/tursodatabase/core/LimboStatementTest.java b/bindings/java/src/test/java/org/github/tursodatabase/core/LimboStatementTest.java new file mode 100644 index 000000000..ea0620a8d --- /dev/null +++ b/bindings/java/src/test/java/org/github/tursodatabase/core/LimboStatementTest.java @@ -0,0 +1,31 @@ +package org.github.tursodatabase.core; + +import static org.junit.jupiter.api.Assertions.*; + +import java.util.Properties; +import org.github.tursodatabase.TestUtils; +import org.github.tursodatabase.jdbc4.JDBC4Connection; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class LimboStatementTest { + + private JDBC4Connection connection; + + @BeforeEach + void setUp() throws Exception { + String filePath = TestUtils.createTempFile(); + String url = "jdbc:sqlite:" + filePath; + connection = new JDBC4Connection(url, filePath, new Properties()); + } + + @Test + void closing_statement_closes_related_resources() throws Exception { + LimboStatement stmt = connection.prepare("SELECT 1"); + stmt.execute(); + + stmt.close(); + assertTrue(stmt.isClosed()); + assertFalse(stmt.getResultSet().isOpen()); + } +} diff --git a/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4StatementTest.java b/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4StatementTest.java index 2a837629d..c9894d2a2 100644 --- a/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4StatementTest.java +++ b/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4StatementTest.java @@ -3,6 +3,7 @@ package org.github.tursodatabase.jdbc4; import static org.junit.jupiter.api.Assertions.*; import java.sql.ResultSet; +import java.sql.SQLException; import java.sql.Statement; import java.util.Properties; import org.github.tursodatabase.TestUtils; @@ -51,4 +52,22 @@ class JDBC4StatementTest { stmt.execute("INSERT INTO users VALUES (1, 'limbo');"); assertTrue(stmt.execute("SELECT * FROM users;")); } + + @Test + void close_statement_test() throws Exception { + stmt.close(); + assertTrue(stmt.isClosed()); + } + + @Test + void double_close_is_no_op() throws SQLException { + stmt.close(); + assertDoesNotThrow(() -> stmt.close()); + } + + @Test + void operations_on_closed_statement_should_throw_exception() throws Exception { + stmt.close(); + assertThrows(SQLException.class, () -> stmt.execute("SELECT 1")); + } } From b17511b55914566e1722f640ebe0266aae5bc358 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=84=A0=EC=9A=B0?= Date: Tue, 28 Jan 2025 09:25:57 +0900 Subject: [PATCH 6/6] Fix test to use lighter query --- .../org/github/tursodatabase/core/LimboStatementTest.java | 2 +- .../github/tursodatabase/jdbc4/JDBC4ResultSetTest.java | 8 ++------ .../github/tursodatabase/jdbc4/JDBC4StatementTest.java | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/bindings/java/src/test/java/org/github/tursodatabase/core/LimboStatementTest.java b/bindings/java/src/test/java/org/github/tursodatabase/core/LimboStatementTest.java index ea0620a8d..fe274b07e 100644 --- a/bindings/java/src/test/java/org/github/tursodatabase/core/LimboStatementTest.java +++ b/bindings/java/src/test/java/org/github/tursodatabase/core/LimboStatementTest.java @@ -21,7 +21,7 @@ class LimboStatementTest { @Test void closing_statement_closes_related_resources() throws Exception { - LimboStatement stmt = connection.prepare("SELECT 1"); + LimboStatement stmt = connection.prepare("SELECT 1;"); stmt.execute(); stmt.close(); diff --git a/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java b/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java index 905b6f0f3..a16c096c9 100644 --- a/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java +++ b/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4ResultSetTest.java @@ -62,9 +62,7 @@ class JDBC4ResultSetTest { @Test void close_resultSet_test() throws Exception { - stmt.executeUpdate("CREATE TABLE users (id INT PRIMARY KEY, username TEXT);"); - stmt.executeUpdate("INSERT INTO users VALUES (2, 'seonwoo');"); - stmt.executeQuery("SELECT * FROM users"); + stmt.executeQuery("SELECT 1;"); ResultSet resultSet = stmt.getResultSet(); assertFalse(resultSet.isClosed()); @@ -74,9 +72,7 @@ class JDBC4ResultSetTest { @Test void calling_methods_on_closed_resultSet_should_throw_exception() throws Exception { - stmt.executeUpdate("CREATE TABLE users (id INT PRIMARY KEY, username TEXT);"); - stmt.executeUpdate("INSERT INTO users VALUES (2, 'seonwoo');"); - stmt.executeQuery("SELECT * FROM users"); + stmt.executeQuery("SELECT 1;"); ResultSet resultSet = stmt.getResultSet(); resultSet.close(); assertTrue(resultSet.isClosed()); diff --git a/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4StatementTest.java b/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4StatementTest.java index c9894d2a2..a48cedea9 100644 --- a/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4StatementTest.java +++ b/bindings/java/src/test/java/org/github/tursodatabase/jdbc4/JDBC4StatementTest.java @@ -68,6 +68,6 @@ class JDBC4StatementTest { @Test void operations_on_closed_statement_should_throw_exception() throws Exception { stmt.close(); - assertThrows(SQLException.class, () -> stmt.execute("SELECT 1")); + assertThrows(SQLException.class, () -> stmt.execute("SELECT 1;")); } }