fix(jdbc): execute statements via Statement.execute() instead of keyword sniffing - #148
Closed
Blankll wants to merge 1 commit into
Closed
fix(jdbc): execute statements via Statement.execute() instead of keyword sniffing#148Blankll wants to merge 1 commit into
Blankll wants to merge 1 commit into
Conversation
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
QueryExecutor(JDBC bridge, used for Oracle and other JDBC-routed databases) chose betweenexecuteQuery()andexecuteUpdate()by uppercasing the SQL and checking a prefix list: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 … RETURNINGon databases that support it never surfaced its returned rows.EXPLAIN, vendor extensions) required maintaining the sniffing list.Fix
Delegate to
Statement.execute()and drain results generically:execute()returnstrue→ read theResultSet→{columns, rows}execute()returnsfalse→ capturegetUpdateCount()→{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 compilepasses (checked against local JDK 21 via-Dmaven.compiler.source/target=21; the pom targets Java 25 for release builds as before)SELECT→ rows, plainINSERT/UPDATE/DELETE→ rows_affected, DDL →rows_affected: 0Out 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.