From 9329e6280559a1f0acdf44fedb4072ff9ab8a2b6 Mon Sep 17 00:00:00 2001 From: Magnus Persson <1431685+magnusp@users.noreply.github.com> Date: Fri, 23 Oct 2026 23:25:28 +0200 Subject: [PATCH 1/2] refactor(jdbc): route statement SQL checks through SqlTokenizer --- .../libsql/jdbc/LibsqlPreparedStatement.java | 114 +++--------------- .../magnusp/libsql/jdbc/LibsqlStatement.java | 93 +++----------- 2 files changed, 32 insertions(+), 175 deletions(-) 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 9ce6b7b..0d037db 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 @@ -31,7 +31,6 @@ import java.util.ArrayList; import java.util.Calendar; import java.util.List; -import java.util.Locale; /** * Prepared statement over the Hrana transport. @@ -105,11 +104,18 @@ public int executeUpdate() throws SQLException { } private static boolean returnsResultSet(String sql) { - List tokens = tokenize(sql); - if (tokens.isEmpty()) { + if (sql == null) { + return false; + } + List tokens = SqlTokenizer.tokenize(sql); + if (tokens == null) { return false; } - return switch (tokens.get(0).word()) { + String first = SqlTokenizer.wordAt(tokens, 0, 0); + if (first == null) { + return false; + } + return switch (first) { case "SELECT", "VALUES", "EXPLAIN" -> true; case "PRAGMA" -> pragmaReturnsResultSet(tokens); case "INSERT", "UPDATE", "DELETE", "REPLACE" -> hasTopLevelReturning(tokens, 1); @@ -118,23 +124,22 @@ private static boolean returnsResultSet(String sql) { }; } - private static boolean pragmaReturnsResultSet(List tokens) { + private static boolean pragmaReturnsResultSet(List tokens) { for (int i = 1; i < tokens.size(); i++) { - Token token = tokens.get(i); - if (token.depth() == 0 && token.word().equals("=")) { + if (SqlTokenizer.isSymbolAt(tokens, i, "=", 0)) { return false; } } return true; } - private static boolean withReturnsResultSet(List tokens) { + private static boolean withReturnsResultSet(List tokens) { for (int i = 1; i < tokens.size(); i++) { - Token token = tokens.get(i); - if (token.depth() != 0) { + String word = SqlTokenizer.wordAt(tokens, i, 0); + if (word == null) { continue; } - switch (token.word()) { + switch (word) { case "SELECT", "VALUES" -> { return true; } @@ -148,98 +153,15 @@ private static boolean withReturnsResultSet(List tokens) { return false; } - private static boolean hasTopLevelReturning(List tokens, int from) { + private static boolean hasTopLevelReturning(List tokens, int from) { for (int i = from; i < tokens.size(); i++) { - Token token = tokens.get(i); - if (token.depth() == 0 && token.word().equals("RETURNING")) { + if (SqlTokenizer.isWordAt(tokens, i, 0, "RETURNING")) { return true; } } return false; } - private static List tokenize(String sql) { - List tokens = new ArrayList<>(); - if (sql == null) { - return tokens; - } - int depth = 0; - int i = 0; - int n = sql.length(); - while (i < n) { - char c = sql.charAt(i); - if (Character.isWhitespace(c)) { - 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 = Math.min(n, i + 2); - } else if (c == '\'' || c == '"' || c == '`') { - i = skipQuoted(sql, i, c); - } else if (c == '[') { - i++; - while (i < n && sql.charAt(i) != ']') { - i++; - } - if (i < n) { - i++; - } - } else if (c == '(') { - depth++; - i++; - } else if (c == ')') { - depth = Math.max(0, depth - 1); - i++; - } else if (c == '=') { - tokens.add(new Token("=", depth)); - i++; - } else if (isWordStart(c)) { - int start = i; - i++; - while (i < n && isWordPart(sql.charAt(i))) { - i++; - } - tokens.add(new Token(sql.substring(start, i).toUpperCase(Locale.ROOT), depth)); - } else { - i++; - } - } - return tokens; - } - - private static int skipQuoted(String sql, int start, char quote) { - int i = start + 1; - int n = sql.length(); - while (i < n) { - if (sql.charAt(i) == quote) { - if (i + 1 < n && sql.charAt(i + 1) == quote) { - i += 2; - continue; - } - return i + 1; - } - i++; - } - return n; - } - - private static boolean isWordStart(char c) { - return Character.isLetter(c) || c == '_'; - } - - private static boolean isWordPart(char c) { - return Character.isLetterOrDigit(c) || c == '_' || c == '$'; - } - - private record Token(String word, int depth) {} - @Override public boolean execute() throws SQLException { return executeResult(sql, executeCurrent()); 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 ea60206..8381178 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 @@ -15,6 +15,7 @@ import java.sql.Statement; import java.util.ArrayList; import java.util.List; +import java.util.Set; /** * Statement over the Hrana transport. @@ -80,6 +81,9 @@ public class LibsqlStatement implements Statement { private int maxRows = 0; private boolean escapeProcessing = true; + private static final Set STATEMENT_KEYWORDS = + Set.of("SELECT", "VALUES", "INSERT", "REPLACE", "UPDATE", "DELETE"); + public LibsqlStatement(LibsqlConnection connection) { this.connection = connection; } @@ -402,89 +406,20 @@ protected void clearGeneratedKeys() throws SQLException { } private static boolean isInsertSql(String sql) { - int start = skipWhitespace(sql, 0); - if (keywordAt(sql, start, "INSERT") || keywordAt(sql, start, "REPLACE")) { - return true; - } - if (!keywordAt(sql, start, "WITH")) { - return false; - } - int afterWith = start + "WITH".length(); - if (afterWith >= sql.length() || !Character.isWhitespace(sql.charAt(afterWith))) { + List tokens = SqlTokenizer.tokenize(sql); + if (tokens == null) { return false; } - - int depth = 0; - int i = afterWith; - while (i < sql.length()) { - char c = sql.charAt(i); - if (c == '-' && i + 1 < sql.length() && sql.charAt(i + 1) == '-') { - i += 2; - while (i < sql.length() && sql.charAt(i) != '\n') { - i++; - } - } else if (c == '/' && i + 1 < sql.length() && sql.charAt(i + 1) == '*') { - i += 2; - while (i + 1 < sql.length() - && !(sql.charAt(i) == '*' && sql.charAt(i + 1) == '/')) { - i++; - } - i = Math.min(sql.length(), i + 2); - } else if (c == '\'' || c == '"' || c == '`') { - i = skipQuoted(sql, i, c); - } else if (c == '[') { - i++; - while (i < sql.length() && sql.charAt(i) != ']') { - i++; - } - if (i < sql.length()) { - i++; - } - } else if (c == '(') { - depth++; - i++; - } else if (c == ')') { - depth = Math.max(0, depth - 1); - i++; - if (depth == 0) { - int next = skipWhitespace(sql, i); - if (keywordAt(sql, next, "INSERT") || keywordAt(sql, next, "REPLACE")) { - return true; - } - } - } else { - i++; - } - } - return false; - } - - private static int skipWhitespace(String sql, int start) { - int i = start; - while (i < sql.length() && Character.isWhitespace(sql.charAt(i))) { - i++; + String first = SqlTokenizer.wordAt(tokens, 0, 0); + if ("INSERT".equals(first) || "REPLACE".equals(first)) { + return true; } - return i; - } - - private static int skipQuoted(String sql, int start, char quote) { - int i = start + 1; - while (i < sql.length()) { - if (sql.charAt(i) == quote) { - if (i + 1 < sql.length() && sql.charAt(i + 1) == quote) { - i += 2; - continue; - } - return i + 1; - } - i++; + if (!"WITH".equals(first)) { + return false; } - return sql.length(); - } - - private static boolean keywordAt(String sql, int index, String keyword) { - return index + keyword.length() <= sql.length() - && sql.regionMatches(true, index, keyword, 0, keyword.length()); + int index = SqlTokenizer.indexOfAnyWord(tokens, 1, tokens.size(), 0, STATEMENT_KEYWORDS); + String statement = SqlTokenizer.wordAt(tokens, index, 0); + return "INSERT".equals(statement) || "REPLACE".equals(statement); } private ResultSet emptyGeneratedKeys() { From e652c87792bc47ab9cc33f6ee0c97906ebd9036b Mon Sep 17 00:00:00 2001 From: Magnus Persson <1431685+magnusp@users.noreply.github.com> Date: Fri, 23 Oct 2026 23:25:28 +0200 Subject: [PATCH 2/2] test(jdbc): pin generated-key and statement-kind detection Cover INSERT OR REPLACE, comments, quoted identifiers, PRAGMA assignment, and malformed input. --- .../libsql/jdbc/LibsqlGeneratedKeysTest.java | 101 +++++++++++++++++ .../jdbc/LibsqlPreparedStatementTest.java | 104 ++++++++++++++++++ 2 files changed, 205 insertions(+) diff --git a/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlGeneratedKeysTest.java b/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlGeneratedKeysTest.java index d20103d..07e550a 100644 --- a/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlGeneratedKeysTest.java +++ b/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlGeneratedKeysTest.java @@ -50,6 +50,107 @@ void autoCommitInsertReturnsLastInsertRowidFromStmtResult() throws SQLException } } + @Test + void insertOrReplaceReturnsGeneratedKey() throws SQLException { + RecordingConnection connection = new RecordingConnection(insertResult(8L)); + + try (Statement statement = connection.createStatement()) { + statement.executeUpdate("insert or replace into k (id, v) values (8, 'a')"); + + try (ResultSet keys = statement.getGeneratedKeys()) { + assertThat(keys.next()).isTrue(); + assertThat(keys.getLong(1)).isEqualTo(8L); + } + } + } + + @Test + void commentsAroundInsertLeaveTheKeyDetectionIntact() throws SQLException { + RecordingConnection connection = new RecordingConnection(insertResult(4L)); + + try (Statement statement = connection.createStatement()) { + statement.executeUpdate("/* seed */ insert into k (v) values ('a')"); + try (ResultSet keys = statement.getGeneratedKeys()) { + assertThat(keys.next()).isTrue(); + assertThat(keys.getLong(1)).isEqualTo(4L); + } + + statement.executeUpdate("-- seed\ninsert into k (v) values ('b')"); + try (ResultSet keys = statement.getGeneratedKeys()) { + assertThat(keys.next()).isTrue(); + assertThat(keys.getLong(1)).isEqualTo(4L); + } + } + } + + @Test + void commentBetweenCteAndInsertLeavesTheKeyDetectionIntact() throws SQLException { + RecordingConnection connection = new RecordingConnection(insertResult(12L)); + + try (Statement statement = connection.createStatement()) { + statement.executeUpdate("with cte as (select lower('--') as v)" + + " /* insert */ insert into k (v) select v from cte"); + + try (ResultSet keys = statement.getGeneratedKeys()) { + assertThat(keys.next()).isTrue(); + assertThat(keys.getLong(1)).isEqualTo(12L); + } + } + } + + @Test + void quotedIdentifierNamedInsertDoesNotCreateGeneratedKeys() throws SQLException { + RecordingConnection connection = new RecordingConnection(insertResult(21L)); + + try (Statement statement = connection.createStatement()) { + statement.executeUpdate("with \"insert\" as (select 1) select * from \"insert\""); + + try (ResultSet keys = statement.getGeneratedKeys()) { + assertThat(keys.next()).isFalse(); + } + } + } + + @Test + void cteUpdateDoesNotCreateGeneratedKeys() throws SQLException { + RecordingConnection connection = new RecordingConnection(insertResult(22L)); + + try (Statement statement = connection.createStatement()) { + statement.executeUpdate("with cte as (select 1) update k set v = 'a'"); + + try (ResultSet keys = statement.getGeneratedKeys()) { + assertThat(keys.next()).isFalse(); + } + } + } + + @Test + void quotedTableNameStillReportsTheInsertKey() throws SQLException { + RecordingConnection connection = new RecordingConnection(insertResult(23L)); + + try (Statement statement = connection.createStatement()) { + statement.executeUpdate("insert into [k] (v) values ('a')"); + + try (ResultSet keys = statement.getGeneratedKeys()) { + assertThat(keys.next()).isTrue(); + assertThat(keys.getLong(1)).isEqualTo(23L); + } + } + } + + @Test + void unbalancedSqlLeavesNoGeneratedKeys() throws SQLException { + RecordingConnection connection = new RecordingConnection(insertResult(24L)); + + try (Statement statement = connection.createStatement()) { + statement.executeUpdate("insert into k (v values ('a')"); + + try (ResultSet keys = statement.getGeneratedKeys()) { + assertThat(keys.next()).isFalse(); + } + } + } + @Test void statementReplaceReturnsGeneratedKey() throws SQLException { RecordingConnection connection = new RecordingConnection(insertResult(7L)); diff --git a/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatementTest.java b/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatementTest.java index aa2626e..aba6cf0 100644 --- a/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatementTest.java +++ b/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatementTest.java @@ -118,6 +118,110 @@ void executeUpdateOnWithSelectThrowsWithoutExecuting() throws SQLException { assertThat(connection.executeCalls).isZero(); } + @Test + void withInsertReturningIsAQuery() throws SQLException { + RecordingConnection connection = new RecordingConnection(ROWS); + try (LibsqlPreparedStatement statement = new LibsqlPreparedStatement( + connection, + "with cte as (select 1 as v) insert into t (v) select v from cte returning id")) { + ResultSet resultSet = statement.executeQuery(); + + assertThat(resultSet.next()).isTrue(); + } + + assertThat(connection.executeCalls).isEqualTo(1); + } + + @Test + void withInsertWithoutReturningIsAnUpdate() throws SQLException { + RecordingConnection connection = new RecordingConnection(NO_ROWS); + try (LibsqlPreparedStatement statement = new LibsqlPreparedStatement( + connection, "with cte as (select 1 as v) insert into t (v) select v from cte")) { + assertThat(statement.executeUpdate()).isEqualTo(1); + } + + assertThat(connection.executeCalls).isEqualTo(1); + } + + @Test + void pragmaWithoutAssignmentIsAQuery() throws SQLException { + RecordingConnection connection = new RecordingConnection(ROWS); + try (LibsqlPreparedStatement statement = + new LibsqlPreparedStatement(connection, "pragma table_info('t')")) { + assertThatThrownBy(statement::executeUpdate) + .isInstanceOf(SQLException.class) + .hasMessage("Query returns results"); + + ResultSet resultSet = statement.executeQuery(); + assertThat(resultSet.next()).isTrue(); + } + + assertThat(connection.executeCalls).isEqualTo(1); + } + + @Test + void pragmaAssignmentIsAnUpdate() throws SQLException { + RecordingConnection connection = new RecordingConnection(NO_ROWS); + try (LibsqlPreparedStatement statement = + new LibsqlPreparedStatement(connection, "pragma journal_mode = wal")) { + assertThatThrownBy(statement::executeQuery) + .isInstanceOf(SQLException.class) + .hasMessage("Query does not return results"); + + assertThat(statement.executeUpdate()).isEqualTo(1); + } + + assertThat(connection.executeCalls).isEqualTo(1); + } + + @Test + void lineCommentBeforeSelectKeepsTheQueryKind() throws SQLException { + RecordingConnection connection = new RecordingConnection(ROWS); + try (LibsqlPreparedStatement statement = + new LibsqlPreparedStatement(connection, "-- insert\nselect id from t")) { + ResultSet resultSet = statement.executeQuery(); + + assertThat(resultSet.next()).isTrue(); + } + + assertThat(connection.executeCalls).isEqualTo(1); + } + + @Test + void blockCommentBeforeUpdateKeepsTheUpdateKind() throws SQLException { + RecordingConnection connection = new RecordingConnection(NO_ROWS); + try (LibsqlPreparedStatement statement = + new LibsqlPreparedStatement(connection, "/* select */ update t set a = 1")) { + assertThat(statement.executeUpdate()).isEqualTo(1); + } + + assertThat(connection.executeCalls).isEqualTo(1); + } + + @Test + void quotedIdentifierNamedLikeAKeywordDoesNotDecideTheStatementKind() throws SQLException { + RecordingConnection connection = new RecordingConnection(ROWS); + try (LibsqlPreparedStatement statement = new LibsqlPreparedStatement( + connection, "with \"update\" as (select id from t) select id from \"update\"")) { + ResultSet resultSet = statement.executeQuery(); + + assertThat(resultSet.next()).isTrue(); + } + + assertThat(connection.executeCalls).isEqualTo(1); + } + + @Test + void malformedSqlIsTreatedAsAnUpdate() throws SQLException { + RecordingConnection connection = new RecordingConnection(NO_ROWS); + try (LibsqlPreparedStatement statement = + new LibsqlPreparedStatement(connection, "select (1")) { + assertThat(statement.executeUpdate()).isEqualTo(1); + } + + assertThat(connection.executeCalls).isEqualTo(1); + } + @Test void setMaxRowsLimitsPreparedQueryAndGetMaxRowsReadsItBack() throws SQLException { RecordingConnection connection = new RecordingConnection(THREE_ROWS);