Repository navigation
fix(jdbc): reset generated keys and result sets on every execution path - #142
Merged
Merged
Conversation
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.
Fixes #106
Contract: Hrana 3 is the source of truth for transport, values, metadata, batching, transactions, and error reporting;
java.sqlis the source of truth for the JDBC surface; xerial/sqlite-jdbc is a behavioral reference and conformance foundation, never a copy target.Base
57e1b53b6ee813ad719a93cb23a05f2af4f70c80, headf0fbdf53fc33648716f19a77ca38728184d9692c.What changed
Every execution path now starts from
beginExecution(): it closes the current result set, discards the generated keys, and resets the update count. The paths areexecuteSql(plainexecute/executeQuery/executeUpdate),executeCurrent(prepared statements), andexecuteBatch.Generated keys come from
StmtResult.last_insert_rowidwith no follow-up query:BatchResultresult. The update count is the total affected rows across the step results.executeQueryon an insert (for exampleINSERT ... RETURNING) reports that statement's key; a non-insert query clears the keys.BatchUpdateExceptionafter the state reset, so no half-populated keys survive.Divergences, with reasons
executeQueryneither clears nor updates generated keys, so it can report a previous statement's key. Cleared here because the issue names that as a bug andjava.sqlkeys belong to the current statement.executeBatchleaves the previous keys and update count in place, and its docs say it cannot return keys for multiple rows. The batch path reports every insert step'slast_insert_rowidand sets the update count to the total affected rows.java.sqlleavesgetUpdateCountunspecified after a batch, so this is a recorded choice.0(the sum of no step results).last_insert_rowid(), one row per key; the no-keys result keeps xerial's1column shape.Files outside the plan's Files block
libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlConnection.java:executeBatchdiscarded theBatchResultstep results and returned onlyint[]. AddedexecuteBatchResultsreturning the step results and madeexecuteBatchdelegate to it. The Build box ("from eachBatchResultstep result") is not implementable without this.libsql-itests/src/test/java/com/github/magnusp/libsql/itest/GeneratedKeysIT.java: the issue says this file has no batch case, and lane 2 runs it. Added the live batch andRETURN_GENERATED_KEYS-batch case.Evidence
Red, trunk (
origin/main, same assertions through the trunk connection seam):LibsqlGeneratedKeysTrunkProbeTest6/6 fail — stale key 5 after batch, stale key 3 afterexecuteQuery RETURNING, stale count 1, batch leaves the previous result set open, failed batch leaves the old key.Log:
/tmp/xerial-parity-i106/lane-1-regression.log.Green at head:
LibsqlGeneratedKeysTest16/16 pass;GeneratedKeysIT4/4 live against sqld, including the new batch case.Lanes (all at head, logs under
/tmp/xerial-parity-i106/):GeneratedKeysITlive lane: 4/4, BUILD SUCCESS (lane-2-specific.log).LibsqlStatementLifecycleTestplus generated keys: pass (lane-3-lifecycle.log).-pl libsql-jdbc test: BUILD SUCCESS (lane-4-module-suite.log).-pl libsql-conformance -am verify: BUILD SUCCESS, no new divergence line (lane-5-conformance.log).-Pxerial-control verify: BUILD SUCCESS, differences recorded above (lane-6-control.log).spotless:check: BUILD SUCCESS (lane-7-spotless.log)../mvnw test: BUILD SUCCESS (lane-8-reactor.log).getGeneratedKeys: pass, no half-populated keys (lane-9-specific.log).-pl libsql-jdbc -am package -DskipTests: BUILD SUCCESS (lane-10-package.log).Perf: three interleaved
time ./mvnw -pl libsql-jdbc testruns per side. Trunk median 2.07 s, head median 2.09 s (+1.0%, within the 10% rule). Raw values under/tmp/xerial-parity-i106/perf-*.txt.Docker note for lane 2:
DOCKER_CONFIG=/tmp/wt-106/libsql-conformance/src/test/docker-config, per AGENTS.md section 4.