From acd3ed3241063bd24e1025d45bdbd3e81cd4a8e1 Mon Sep 17 00:00:00 2001 From: Magnus Persson <1431685+magnusp@users.noreply.github.com> Date: Sat, 24 Oct 2026 20:04:47 +0200 Subject: [PATCH] refactor(jdbc): unify statement result attachment Attach the current result through one `attachResult` transition shared by `assignResultSet` and `executeResult`, and discard it through `discardCurrentResult` when close won the race. The closed branch now closes the discarded result set with the new `LibsqlResultSet.closeDetached`, so `closeResultSet` no longer flips `closed`/`closing` to let a nested close through. `newQueryResult` is shared by both `executeQuery` paths. `JdbcEscapes` holds the JDBC escape scan that `checkEscapeProcessing` used to inline; the move keeps `LibsqlStatement` under 650 lines. --- .../magnusp/libsql/jdbc/JdbcEscapes.java | 71 ++++++++ .../libsql/jdbc/LibsqlPreparedStatement.java | 2 +- .../magnusp/libsql/jdbc/LibsqlResultSet.java | 11 +- .../magnusp/libsql/jdbc/LibsqlStatement.java | 154 +++++------------- 4 files changed, 126 insertions(+), 112 deletions(-) create mode 100644 libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/JdbcEscapes.java diff --git a/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/JdbcEscapes.java b/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/JdbcEscapes.java new file mode 100644 index 0000000..55e204b --- /dev/null +++ b/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/JdbcEscapes.java @@ -0,0 +1,71 @@ +package com.github.magnusp.libsql.jdbc; + +/** + * Finds JDBC escape sequences in SQL text. + * + *

The driver sends SQL to sqld verbatim, so a sequence such as {@code {fn abs(-1)}} would fail as + * SQLite syntax instead of being translated. Only a real escape needs rejecting: braces inside + * string literals, quoted identifiers, and comments do not count, and a brace that does not start + * with an escape code is left to the server. + */ +final class JdbcEscapes { + + private JdbcEscapes() {} + + /** + * Whether the SQL contains a JDBC escape sequence. A null or brace-free string contains none. + */ + static boolean contains(String sql) { + if (sql == null || sql.indexOf('{') < 0) { + return false; + } + int n = sql.length(); + int i = 0; + while (i < n) { + char c = sql.charAt(i); + if (c == '\'' || c == '"' || c == '`') { + i++; + while (i < n) { + if (sql.charAt(i) == c) { + if (i + 1 < n && sql.charAt(i + 1) == c) { + i += 2; + continue; + } + i++; + break; + } + i++; + } + } else if (c == '[') { + i++; + while (i < n && sql.charAt(i) != ']') { + i++; + } + i++; + } else if (c == '-' && i + 1 < n && sql.charAt(i + 1) == '-') { + i += 2; + while (i < n && sql.charAt(i) != '\n') { + i++; + } + } else if (c == '/' && i + 1 < n && sql.charAt(i + 1) == '*') { + i += 2; + while (i + 1 < n && !(sql.charAt(i) == '*' && sql.charAt(i + 1) == '/')) { + i++; + } + i += 2; + } else if (c == '{') { + int code = i + 1; + while (code < n && Character.isWhitespace(sql.charAt(code))) { + code++; + } + if (code < n && "dDtTfFoOeEcC".indexOf(sql.charAt(code)) >= 0) { + return true; + } + i++; + } else { + i++; + } + } + return false; + } +} diff --git a/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatement.java b/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatement.java index 9f221f2..a3fb301 100644 --- a/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatement.java +++ b/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatement.java @@ -88,7 +88,7 @@ public ResultSet executeQuery() throws SQLException { if (result.cols() == null || result.cols().isEmpty()) { throw new SQLException("Query does not return results"); } - LibsqlResultSet resultSet = new LibsqlResultSet(this, limitRows(result), sql, false); + LibsqlResultSet resultSet = newQueryResult(sql, result); assignResultSet(resultSet, result, sql, true); return resultSet; } diff --git a/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlResultSet.java b/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlResultSet.java index 68a932e..10955b5 100644 --- a/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlResultSet.java +++ b/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlResultSet.java @@ -135,7 +135,7 @@ public void close() throws SQLException { if (closed) { return; } - this.closed = true; + closeDetached(); if (statement != null && (closeStatementOnClose || (statement instanceof LibsqlStatement s && s.isCloseOnCompletion()))) { @@ -143,6 +143,15 @@ public void close() throws SQLException { } } + /** + * Marks this result set closed without calling back into the statement. The owning statement + * calls this while it already holds its own lock, so closing a result set never re-enters it and + * never observes the statement as reopened. + */ + void closeDetached() { + this.closed = true; + } + @Override public boolean wasNull() throws SQLException { checkClosed(); 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 8381178..7fad62c 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 @@ -111,11 +111,16 @@ public ResultSet executeQuery(String sql) throws SQLException { if (result.cols() == null || result.cols().isEmpty()) { throw new SQLException("Query did not produce a ResultSet: " + sql); } - LibsqlResultSet resultSet = new LibsqlResultSet(this, limitRows(result), sql, false); + LibsqlResultSet resultSet = newQueryResult(sql, result); assignResultSet(resultSet, result, sql, true); return resultSet; } + /** Builds the result set both query paths attach, so the two cannot drift apart. */ + protected LibsqlResultSet newQueryResult(String sql, StmtResult result) { + return new LibsqlResultSet(this, limitRows(result), sql, false); + } + protected void assignResultSet(LibsqlResultSet resultSet, boolean requested) throws SQLException { assignResultSet(resultSet, null, null, requested); @@ -124,22 +129,7 @@ protected void assignResultSet(LibsqlResultSet resultSet, boolean requested) protected void assignResultSet( LibsqlResultSet resultSet, StmtResult result, String sql, boolean requested) throws SQLException { - if (!isClosed()) { - beforeResultSetAssignment(); - } - synchronized (stateLock) { - if (closed) { - closeResultSet(resultSet); - currentResultSet = null; - resultSetRequested = false; - updateCount = -1; - return; - } - updateGeneratedKeys(sql, result); - currentResultSet = resultSet; - updateCount = -1; - resultSetRequested = requested; - } + attachResult(resultSet, result, sql, requested); } @Override @@ -155,32 +145,50 @@ public boolean execute(String sql) throws SQLException { protected boolean executeResult(String sql, StmtResult result) throws SQLException { boolean hasResultSet = result.cols() != null && !result.cols().isEmpty(); - LibsqlResultSet resultSet = - hasResultSet ? new LibsqlResultSet(this, limitRows(result), sql, false) : null; + LibsqlResultSet resultSet = hasResultSet ? newQueryResult(sql, result) : null; + return attachResult(resultSet, result, sql, false); + } - if (hasResultSet && !isClosed()) { + /** + * Attaches {@code resultSet} as the current result in one transition shared by the explicit + * {@code executeQuery} paths and the {@code execute} paths. When close wins the race, the result + * set is discarded instead of attached, so a result set produced after {@link #close()} is + * handed out closed and the statement stays closed. + */ + protected boolean attachResult( + LibsqlResultSet resultSet, StmtResult result, String sql, boolean requested) + throws SQLException { + if (resultSet != null && !isClosed()) { beforeResultSetAssignment(); } - synchronized (stateLock) { if (closed) { - clearGeneratedKeys(); - if (resultSet != null) { - closeResultSet(resultSet); - } - currentResultSet = null; - resultSetRequested = false; - updateCount = hasResultSet ? -1 : result.affectedRowCount(); - return hasResultSet; + discardCurrentResult(resultSet, resultSet == null ? result.affectedRowCount() : -1); + return resultSet != null; } updateGeneratedKeys(sql, result); currentResultSet = resultSet; - updateCount = hasResultSet ? -1 : result.affectedRowCount(); - resultSetRequested = false; - return hasResultSet; + updateCount = resultSet == null ? result.affectedRowCount() : -1; + resultSetRequested = requested; + return resultSet != null; } } + /** + * Clears the current result because the statement is closed. The result set closes without the + * close-on-completion callback, and nothing temporarily reopens the statement. + */ + private void discardCurrentResult(LibsqlResultSet resultSet, long updateCountWhenNoResultSet) + throws SQLException { + if (resultSet != null) { + resultSet.closeDetached(); + } + currentResultSet = null; + resultSetRequested = false; + updateCount = updateCountWhenNoResultSet; + clearGeneratedKeys(); + } + private StmtResult executeSql(String sql) throws SQLException { checkEscapeProcessing(sql); beginExecution(); @@ -218,70 +226,11 @@ protected void checkEscapeProcessing(String sql) throws SQLException { synchronized (stateLock) { enabled = escapeProcessing; } - if (enabled && hasEscapeSequence(sql)) { + if (enabled && JdbcEscapes.contains(sql)) { throw new SQLFeatureNotSupportedException("JDBC escape sequences are not supported"); } } - /** - * Whether the SQL contains a JDBC escape sequence. Braces inside string literals, quoted - * identifiers, and comments do not count, and a brace that does not start with an escape code - * is left to the server. - */ - private static boolean hasEscapeSequence(String sql) { - if (sql == null || sql.indexOf('{') < 0) { - return false; - } - int n = sql.length(); - int i = 0; - while (i < n) { - char c = sql.charAt(i); - if (c == '\'' || c == '"' || c == '`') { - i++; - while (i < n) { - if (sql.charAt(i) == c) { - if (i + 1 < n && sql.charAt(i + 1) == c) { - i += 2; - continue; - } - i++; - break; - } - i++; - } - } else if (c == '[') { - i++; - while (i < n && sql.charAt(i) != ']') { - i++; - } - i++; - } else if (c == '-' && i + 1 < n && sql.charAt(i + 1) == '-') { - i += 2; - while (i < n && sql.charAt(i) != '\n') { - i++; - } - } else if (c == '/' && i + 1 < n && sql.charAt(i + 1) == '*') { - i += 2; - while (i + 1 < n && !(sql.charAt(i) == '*' && sql.charAt(i + 1) == '/')) { - i++; - } - i += 2; - } else if (c == '{') { - int code = i + 1; - while (code < n && Character.isWhitespace(sql.charAt(code))) { - code++; - } - if (code < n && "dDtTfFoOeEcC".indexOf(sql.charAt(code)) >= 0) { - return true; - } - i++; - } else { - i++; - } - } - return false; - } - /** * Drops rows above the {@link #setMaxRows(int)} limit. java.sql requires the excess rows to be * dropped silently from the result set, not from the statement that produced it, so the server @@ -321,22 +270,7 @@ protected void closeCurrentResultSet() throws SQLException { currentResultSet = null; resultSetRequested = false; if (resultSet != null) { - closeResultSet(resultSet); - } - } - } - - private void closeResultSet(LibsqlResultSet resultSet) throws SQLException { - synchronized (stateLock) { - boolean wasClosed = closed; - boolean wasClosing = closing; - closed = false; - closing = true; - try { - resultSet.close(); - } finally { - closed = wasClosed; - closing = wasClosing; + resultSet.closeDetached(); } } } @@ -398,7 +332,7 @@ protected void clearGeneratedKeys() throws SQLException { ResultSet keys = generatedKeys; generatedKeys = null; if (keys instanceof LibsqlResultSet libsqlKeys) { - closeResultSet(libsqlKeys); + libsqlKeys.closeDetached(); } else if (keys != null) { keys.close(); } @@ -541,7 +475,7 @@ public void close() throws SQLException { resultSetRequested = false; clearGeneratedKeys(); if (resultSet != null) { - closeResultSet(resultSet); + resultSet.closeDetached(); } } finally { closed = true;