From 430adef1c5d582804ac2746f9aeeed4b27c1088d Mon Sep 17 00:00:00 2001 From: Magnus Persson <1431685+magnusp@users.noreply.github.com> Date: Thu, 22 Oct 2026 01:36:19 +0200 Subject: [PATCH] fix(jdbc): match xerial statement lifecycle transitions --- .../magnusp/libsql/jdbc/LibsqlStatement.java | 117 +++++++++++-- .../jdbc/LibsqlStatementLifecycleTest.java | 154 ++++++++++++++++++ 2 files changed, 256 insertions(+), 15 deletions(-) create mode 100644 libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlStatementLifecycleTest.java diff --git a/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlStatement.java b/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlStatement.java index 5393cef..4a5e2b5 100644 --- a/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlStatement.java +++ b/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlStatement.java @@ -9,11 +9,37 @@ import java.sql.Connection; import java.sql.ResultSet; import java.sql.SQLException; +import java.sql.SQLFeatureNotSupportedException; import java.sql.SQLWarning; import java.sql.Statement; import java.util.ArrayList; import java.util.List; +/** + * Statement over the Hrana transport. + * + *

The lifecycle matches sqlite-jdbc 3.53.4.0 rather than strict JDBC, because the driver is a + * drop-in replacement for that driver: + * + *

+ */ public class LibsqlStatement implements Statement { protected final LibsqlConnection connection; @@ -23,6 +49,8 @@ public class LibsqlStatement implements Statement { protected boolean closeOnCompletion = false; protected final List batchStatements = new ArrayList<>(); private ResultSet generatedKeys = null; + private boolean resultSetRequested = false; + private boolean closing = false; public LibsqlStatement(LibsqlConnection connection) { this.connection = connection; @@ -32,7 +60,9 @@ protected void checkClosed() throws SQLException { if (closed) { throw new SQLException("Statement is closed"); } - connection.checkClosed(); + if (connection != null) { + connection.checkClosed(); + } } @Override @@ -43,6 +73,7 @@ public ResultSet executeQuery(String sql) throws SQLException { } this.currentResultSet = new LibsqlResultSet(this, result, sql, false); this.updateCount = -1; + this.resultSetRequested = true; return currentResultSet; } @@ -59,25 +90,40 @@ public boolean execute(String sql) throws SQLException { if (result.cols() != null && !result.cols().isEmpty()) { this.currentResultSet = new LibsqlResultSet(this, result, sql, false); this.updateCount = -1; + this.resultSetRequested = false; return true; } else { this.currentResultSet = null; this.updateCount = result.affectedRowCount(); + this.resultSetRequested = false; return false; } } private StmtResult executeSql(String sql) throws SQLException { - checkClosed(); closeCurrentResultSet(); - checkClosed(); return connection.executeStatement(new Stmt(sql)); } protected void closeCurrentResultSet() throws SQLException { - if (currentResultSet != null) { - currentResultSet.close(); - currentResultSet = null; + LibsqlResultSet resultSet = currentResultSet; + currentResultSet = null; + resultSetRequested = false; + if (resultSet != null) { + closeResultSet(resultSet); + } + } + + private void closeResultSet(LibsqlResultSet resultSet) throws SQLException { + boolean wasClosed = closed; + boolean wasClosing = closing; + closed = false; + closing = true; + try { + resultSet.close(); + } finally { + closed = wasClosed; + closing = wasClosing; } } @@ -199,6 +245,13 @@ private ResultSet emptyGeneratedKeys() { @Override public ResultSet getResultSet() throws SQLException { checkClosed(); + if (currentResultSet == null) { + return null; + } + if (resultSetRequested) { + throw new SQLException("ResultSet already requested"); + } + resultSetRequested = true; return currentResultSet; } @@ -246,19 +299,27 @@ public int[] executeBatch() throws SQLException { @Override public Connection getConnection() throws SQLException { - checkClosed(); return connection; } @Override public void close() throws SQLException { - if (!closed) { - closed = true; + if (closing) { + return; + } + closing = true; + try { + closed = false; clearGeneratedKeys(); - if (currentResultSet != null) { - currentResultSet.close(); - currentResultSet = null; + LibsqlResultSet resultSet = currentResultSet; + currentResultSet = null; + resultSetRequested = false; + if (resultSet != null) { + resultSet.close(); } + } finally { + closed = true; + closing = false; } } @@ -284,7 +345,18 @@ public boolean isClosed() throws SQLException { @Override public int getFetchSize() throws SQLException { return 0; } @Override public int getResultSetConcurrency() throws SQLException { return ResultSet.CONCUR_READ_ONLY; } @Override public int getResultSetType() throws SQLException { return ResultSet.TYPE_FORWARD_ONLY; } - @Override public boolean getMoreResults(int current) throws SQLException { return getMoreResults(); } + @Override + public boolean getMoreResults(int current) throws SQLException { + checkClosed(); + if (current == Statement.KEEP_CURRENT_RESULT || current == Statement.CLOSE_ALL_RESULTS) { + throw new SQLFeatureNotSupportedException( + "Argument not supported: Statement.KEEP_CURRENT_RESULT or Statement.CLOSE_ALL_RESULTS"); + } + if (current != Statement.CLOSE_CURRENT_RESULT) { + throw new SQLException("Invalid argument"); + } + return getMoreResults(); + } @Override public ResultSet getGeneratedKeys() throws SQLException { checkClosed(); @@ -302,8 +374,23 @@ public ResultSet getGeneratedKeys() throws SQLException { @Override public int getResultSetHoldability() throws SQLException { return ResultSet.HOLD_CURSORS_OVER_COMMIT; } @Override public void setPoolable(boolean poolable) throws SQLException {} @Override public boolean isPoolable() throws SQLException { return false; } - @Override public void closeOnCompletion() throws SQLException { closeOnCompletion = true; } - @Override public boolean isCloseOnCompletion() throws SQLException { return closeOnCompletion; } + @Override + public void closeOnCompletion() throws SQLException { + checkClosedForCompletion(); + closeOnCompletion = true; + } + + @Override + public boolean isCloseOnCompletion() throws SQLException { + checkClosedForCompletion(); + return closeOnCompletion; + } + + private void checkClosedForCompletion() throws SQLException { + if (closed) { + throw new SQLException("statement is closed"); + } + } @Override public T unwrap(Class iface) throws SQLException { diff --git a/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlStatementLifecycleTest.java b/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlStatementLifecycleTest.java new file mode 100644 index 0000000..c3c0037 --- /dev/null +++ b/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlStatementLifecycleTest.java @@ -0,0 +1,154 @@ +package com.github.magnusp.libsql.jdbc; + +import com.github.magnusp.libsql.hrana.codec.Col; +import com.github.magnusp.libsql.hrana.codec.Row; +import com.github.magnusp.libsql.hrana.codec.Stmt; +import com.github.magnusp.libsql.hrana.codec.StmtResult; +import com.github.magnusp.libsql.hrana.codec.Value; +import org.junit.jupiter.api.Test; + +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.SQLFeatureNotSupportedException; +import java.sql.Statement; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class LibsqlStatementLifecycleTest { + + private static final StmtResult ROWS = new StmtResult( + List.of(new Col("value", null)), + List.of(new Row(List.of(Value.of(1L)))), + 0, + null); + + @Test + void getResultSetAfterExecuteQueryThrowsResultSetAlreadyRequested() throws SQLException { + RecordingConnection connection = new RecordingConnection(ROWS); + + try (Statement statement = connection.createStatement()) { + statement.executeQuery("select value from t"); + + assertThatThrownBy(statement::getResultSet) + .isInstanceOf(SQLException.class) + .hasMessage("ResultSet already requested"); + } + } + + @Test + void getResultSetAfterExecuteReturnsTheResultOnceThenThrows() throws SQLException { + RecordingConnection connection = new RecordingConnection(ROWS); + + try (Statement statement = connection.createStatement()) { + assertThat(statement.execute("select value from t")).isTrue(); + + ResultSet resultSet = statement.getResultSet(); + assertThat(resultSet).isNotNull(); + assertThat(resultSet.next()).isTrue(); + assertThat(resultSet.getLong(1)).isEqualTo(1L); + + assertThatThrownBy(statement::getResultSet) + .isInstanceOf(SQLException.class) + .hasMessage("ResultSet already requested"); + } + } + + @Test + void getMoreResultsRejectsKeepCurrentResult() throws SQLException { + RecordingConnection connection = new RecordingConnection(ROWS); + + try (Statement statement = connection.createStatement()) { + statement.executeQuery("select value from t"); + + assertThatThrownBy(() -> statement.getMoreResults(Statement.KEEP_CURRENT_RESULT)) + .isInstanceOf(SQLFeatureNotSupportedException.class) + .hasMessageContaining("KEEP_CURRENT_RESULT"); + } + } + + @Test + void getMoreResultsRejectsCloseAllResults() throws SQLException { + RecordingConnection connection = new RecordingConnection(ROWS); + + try (Statement statement = connection.createStatement()) { + statement.executeQuery("select value from t"); + + assertThatThrownBy(() -> statement.getMoreResults(Statement.CLOSE_ALL_RESULTS)) + .isInstanceOf(SQLFeatureNotSupportedException.class) + .hasMessageContaining("CLOSE_ALL_RESULTS"); + } + } + + @Test + void getMoreResultsAcceptsCloseCurrentResult() throws SQLException { + RecordingConnection connection = new RecordingConnection(ROWS); + + try (Statement statement = connection.createStatement()) { + statement.executeQuery("select value from t"); + + assertThat(statement.getMoreResults(Statement.CLOSE_CURRENT_RESULT)).isFalse(); + assertThat(statement.getResultSet()).isNull(); + } + } + + @Test + void closeOnCompletionAndIsCloseOnCompletionThrowAfterClose() throws SQLException { + LibsqlStatement statement = new LibsqlStatement(null); + statement.closeOnCompletion(); + assertThat(statement.isCloseOnCompletion()).isTrue(); + + statement.close(); + + assertThatThrownBy(statement::closeOnCompletion) + .isInstanceOf(SQLException.class) + .hasMessage("statement is closed"); + assertThatThrownBy(statement::isCloseOnCompletion) + .isInstanceOf(SQLException.class) + .hasMessage("statement is closed"); + } + + @Test + void executeAndGetConnectionStillWorkAfterClose() throws SQLException { + RecordingConnection connection = new RecordingConnection(ROWS); + Statement statement = connection.createStatement(); + statement.close(); + + assertThat(statement.isClosed()).isTrue(); + assertThat(statement.execute("select value from t")).isTrue(); + assertThat(statement.getConnection()).isSameAs(connection); + assertThat(connection.executeCalls).isEqualTo(1); + + statement.close(); + } + + @Test + void preparedStatementExecuteAfterCloseThrows() throws SQLException { + RecordingConnection connection = new RecordingConnection(ROWS); + LibsqlPreparedStatement statement = + new LibsqlPreparedStatement(connection, "select value from t"); + statement.close(); + + assertThatThrownBy(statement::execute) + .isInstanceOf(SQLException.class) + .hasMessage("Statement is closed"); + assertThat(connection.executeCalls).isZero(); + } + + private static final class RecordingConnection extends LibsqlConnection { + private final StmtResult result; + private int executeCalls; + + private RecordingConnection(StmtResult result) { + super(null, null); + this.result = result; + } + + @Override + public StmtResult executeStatement(Stmt stmt) { + executeCalls++; + return result; + } + } +}