Skip to content

fix(jdbc): execute statements via Statement.execute() instead of keyword sniffing - #148

Closed
Blankll wants to merge 1 commit into
masterfrom
fix/jdbc-execute
Closed

fix(jdbc): execute statements via Statement.execute() instead of keyword sniffing#148
Blankll wants to merge 1 commit into
masterfrom
fix/jdbc-execute

Conversation

@Blankll

@Blankll Blankll commented Sep 3, 2026

Copy link
Copy Markdown
Member

Problem

QueryExecutor (JDBC bridge, used for Oracle and other JDBC-routed databases) chose between executeQuery() and executeUpdate() by uppercasing the SQL and checking a prefix list:

isQuery = upper.startsWith("SELECT") || upper.startsWith("WITH") || ...

Consequences:

  • WITH … INSERT/UPDATE/DELETE (data-modifying CTEs — see fix: data-modifying WITH statements misrouted across SQLKit (classifier, adapters, JDBC bridge) #147 for the Rust-side equivalent bug) was treated as a row-returning query. On drivers that reject statements without a result set (SQL Server, Oracle) executeQuery() throws; on others it runs but reports nothing.
  • INSERT … RETURNING on databases that support it never surfaced its returned rows.
  • Every new statement shape (CTE reads, EXPLAIN, vendor extensions) required maintaining the sniffing list.

Fix

Delegate to Statement.execute() and drain results generically:

  • execute() returns true → read the ResultSet{columns, rows}
  • execute() returns false → capture getUpdateCount(){rows_affected}
  • getMoreResults() loop handles multi-result streams (e.g. SQL Server rowcount + result-set sequences)

The keyword heuristic is removed entirely — statement routing is now decided by the driver's actual protocol response, not by SQL text prefix.

Verification

  • mvn compile passes (checked against local JDK 21 via -Dmaven.compiler.source/target=21; the pom targets Java 25 for release builds as before)
  • Behavior contract unchanged for the previously-working paths: plain SELECT → rows, plain INSERT/UPDATE/DELETE → rows_affected, DDL → rows_affected: 0

Out of scope

Runtime smoke tests against live Oracle/SQL Server instances were not possible in this environment — recommend a manual pass with a JDBC-routed connection before release.

…ord sniffing

QueryExecutor chose executeQuery vs executeUpdate by uppercasing the SQL
and checking a prefix list. Any statement starting with WITH was treated as
a row-returning query, so data-modifying WITH statements (or plain DML with
a leading CTE) hit executeQuery() — drivers that reject statements without a
result set (SQL Server, Oracle) threw, and others returned rows_affected=0.

Delegate to Statement.execute() and drain results with getResultSet()/
getUpdateCount()/getMoreResults(): row-returning statements (SELECT,
WITH ... SELECT, INSERT ... RETURNING) yield columns+rows; everything else
yields rows_affected. The heuristic is removed entirely.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
@Blankll Blankll closed this Sep 3, 2026
@Blankll
Blankll deleted the fix/jdbc-execute branch September 3, 2026 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant