Skip to content

fix(jdbc): match xerial statement lifecycle transitions - #85

Merged
magnusp merged 1 commit into
mainfrom
fix/47-statement-lifecycle
Oct 21, 2026
Merged

magnusp merged 1 commit into
mainfrom
fix/47-statement-lifecycle

Conversation

@magnusp

@magnusp magnusp commented Oct 21, 2026

Copy link
Copy Markdown
Owner

Closes #47. Part of #54.

What changed

  • LibsqlStatement.getResultSet() now hands out the current result set once. It throws SQLException("ResultSet already requested") when a result set was already requested, including the result returned by executeQuery. execute leaves the result available for one getResultSet call.
  • LibsqlStatement.getMoreResults(int) keeps xerial's argument validation. KEEP_CURRENT_RESULT and CLOSE_ALL_RESULTS throw SQLFeatureNotSupportedException("Argument not supported: Statement.KEEP_CURRENT_RESULT or Statement.CLOSE_ALL_RESULTS"). Any other value except CLOSE_CURRENT_RESULT throws SQLException("Invalid argument").
  • closeOnCompletion() and isCloseOnCompletion() throw SQLException("statement is closed") after close, matching xerial's wording and closed-state rule.
  • execute, executeQuery, executeUpdate, and getConnection still work after close(), matching xerial.
  • close() now cleans up the current result set and generated keys even if the statement was closed first, while remaining idempotent.
  • Added class-level Javadoc documenting the lifecycle, and LibsqlStatementLifecycleTest with one behavior test per transition.

Contract decision

Parity with xerial 3.53.4.0 at the pinned tag (JDBC3Statement.java:163-167,378-386, JDBC4Statement.java:34-44):

Transition Behavior
getResultSet() after executeQuery or a prior getResultSet SQLException: ResultSet already requested
getResultSet() after execute Returns the result once; the next call throws
getMoreResults(KEEP_CURRENT_RESULT) / getMoreResults(CLOSE_ALL_RESULTS) SQLFeatureNotSupportedException naming both arguments
getMoreResults(CLOSE_CURRENT_RESULT) Closes the result set, returns false
closeOnCompletion / isCloseOnCompletion after close SQLException: statement is closed
Execute-style methods and getConnection after close Still work

Parity callout: execute-after-close deliberately copies what is probably an xerial bug (JDBC3Statement.internalClose no-ops once the pointer is closed, then prepares again). It is kept because #54 makes parity the default and Hrana does not prevent it. A probe against org.xerial:sqlite-jdbc:3.53.4.0 confirmed that Statement.execute, Statement.executeQuery, Statement.executeUpdate, and getConnection work after close, while prepared-statement execution still throws.

Evidence

Failing before the fix, with the new test file added and production code unchanged:

$ ./mvnw -q -pl libsql-jdbc -am -Dtest=LibsqlStatementLifecycleTest -Dsurefire.failIfNoSpecifiedTests=false test
[ERROR] Tests run: 8, Failures: 5, Errors: 1, Skipped: 0
[ERROR]   closeOnCompletionAndIsCloseOnCompletionThrowAfterClose:104 Expecting code to raise a throwable.
[ERROR]   getMoreResultsRejectsCloseAllResults:78 Expecting code to raise a throwable.
[ERROR]   getMoreResultsRejectsKeepCurrentResult:65 Expecting code to raise a throwable.
[ERROR]   getResultSetAfterExecuteQueryThrowsResultSetAlreadyRequested:34 Expecting code to raise a throwable.
[ERROR]   getResultSetAfterExecuteReturnsTheResultOnceThenThrows:52 Expecting code to raise a throwable.
[ERROR]   executeAndGetConnectionStillWorkAfterClose:119 » SQL Statement is closed

Passing after the fix:

$ ./mvnw -pl libsql-jdbc -am test
[INFO] Tests run: 156, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS

Formatting:

$ ./mvnw spotless:apply
[INFO] BUILD SUCCESS

A direct probe of the published xerial 3.53.4.0 jar reproduced the pinned messages: ResultSet already requested, the KEEP_CURRENT_RESULT/CLOSE_ALL_RESULTS SQLFeatureNotSupportedException message, statement is closed for the completion properties, successful execute/getConnection after close, and prepared execution failing after close.

Deviations

None from xerial for the transitions in scope.

@magnusp
magnusp force-pushed the fix/47-statement-lifecycle branch from 4632e2d to 430adef Compare October 21, 2026 23:39
@magnusp
magnusp merged commit 4b30b46 into main Oct 21, 2026
5 checks passed
@magnusp
magnusp deleted the fix/47-statement-lifecycle branch October 21, 2026 23:43
magnusp added a commit that referenced this pull request Oct 22, 2026
Fixes #107

`LibsqlPreparedStatement.executeQuery()` now passes `true` for the
result-set-requested flag, matching the `LibsqlStatement.executeQuery()`
contract from #85. `ps.executeQuery(); ps.getResultSet()` now throws
`ResultSet already requested`, as sqlite-jdbc does.

**Three-layer contract.** Protocol-neutral: JDBC owns the
result-set-requested lifecycle, and Hrana 3 supplies no field for it.

**Files.** `LibsqlPreparedStatement.java` and
`LibsqlStatementLifecycleTest.java`.

**Red.** At trunk 91ad4c3 with the new case applied: `Tests run: 9,
Failures: 1` and the prepared case fails on `getResultSet()` returning
the open result set instead of throwing
(`/tmp/xerial-parity-i107/lane-1-trunk-red.log`).

**Green.** At head 7125986: `Tests run: 9, Failures: 0`
(`/tmp/xerial-parity-i107/lane-1-unit-green.log`).

**Lanes at head 7125986.**

- Lane 1 regression, trunk red and head green: pass.
- Lane 2 full lifecycle class: pass.
- Lane 3 `ResultSetWithoutResultsTest`: 1 test, pass.
- Lane 4 module suite: 246 tests, BUILD SUCCESS.
- Lane 5 conformance: 123 tests, 0 failures, 1 skipped, BUILD SUCCESS
with no new divergence line.
- Lane 6 xerial control: 100 tests, 0 failures, 1 skipped, BUILD
SUCCESS. No recorded difference; this change has no xerial-side
divergence.
- Lane 7 `spotless:check`: pass.
- Lane 8 full reactor: BUILD SUCCESS.
- Lane 9 plain-statement lifecycle cases: 7 tests, pass.
- Lane 10 package: driver jar builds.

**Perf.** Wall clock of `./mvnw -pl libsql-jdbc test`, three interleaved
runs each. Trunk median 2.10 s, head median 2.08 s, within the 10
percent rule. The first run of each side is cold-cache at 3.11 s and
3.37 s and was excluded by the median.

**Divergences.** None.

**Base.** 91ad4c3. **Head.** 7125986.

Logs are under `/tmp/xerial-parity-i107/`.
magnusp added a commit that referenced this pull request Oct 23, 2026
Fixes #81

Contract: Hrana 3 is the source of truth for transport, values,
metadata, batching, transactions, and error reporting; `java.sql` is the
source of truth for the JDBC surface; xerial/sqlite-jdbc is a behavioral
reference and conformance foundation, never a copy target.

## What changed

`LibsqlPreparedStatement.executeCurrent()` called `checkClosed();
beginExecution(); checkClosed();`. The second check runs after the state
transition and before `connection.executeStatement`, which already
checks the connection itself. Removed it, leaving the single
`checkClosed()` before the state transition.

One file changed, one line deleted, no behavior change.

## Divergence from the plan's Files block

`LibsqlStatement.java` is in the plan's Files block but needed no edit:
its second `checkClosed()` was already removed in `4b30b46` (#85), which
matched xerial's tolerance of `execute` after `statement.close()`. That
behavior is pinned by
`LibsqlStatementLifecycleTest.executeAndGetConnectionStillWorkAfterClose`
(#148), so adding a `checkClosed()` to the statement execute path would
be a behavior change and was not done. `executeSql` reads
`beginExecution();` with the connection check inside
`LibsqlConnection.executeStatement`.

## Evidence

Red at trunk (`origin/main` `de31dc6`): `executeCurrent` at lines
231-235 reads `checkClosed(); beginExecution(); checkClosed();`. Green
at head (`2621b71`): one check. Log:
`/tmp/xerial-parity-i81/lane-3-specific.log`.

Lanes at head (logs under `/tmp/xerial-parity-i81/`):

1. Regression lane: not applicable. The plan's unit box says no new
tests, so there is no new failing case; the diff check stands in for it
(lane 3).
2. Lifecycle and prepared suites, trunk vs head: identical, 21
`LibsqlStatementLifecycleTest` + 6 `LibsqlPreparedStatementTest` = 27/27
pass on both sides (`trunk-unit.log`, `head-unit.log`).
3. No double check remains in either file (`lane-3-specific.log`).
4. `-pl libsql-jdbc test`: BUILD SUCCESS, 293/293
(`lane-4-module-suite.log`); trunk is also 293/293
(`trunk-module-suite.log`).
5. `-pl libsql-conformance -am verify`: BUILD SUCCESS, 123 tests, 0
failures, 1 skipped, no divergence line (`lane-5-conformance.log`).
6. `-Pxerial-control verify`: BUILD SUCCESS, 100 tests, 0 failures, 1
skipped; differences are the pre-existing reference notes, none
introduced here (`lane-6-control.log`).
7. `spotless:check`: BUILD SUCCESS (`lane-7-spotless.log`).
8. `./mvnw test`: BUILD SUCCESS, 293/293 in `libsql-jdbc`
(`lane-8-reactor.log`).
9. Statement surface: BUILD SUCCESS, 30/30 (`lane-9-specific.log`).
10. `-pl libsql-jdbc -am package -DskipTests`: BUILD SUCCESS, driver jar
built (`lane-10-package.log`).

Perf: three interleaved `./mvnw -pl libsql-jdbc test -q` runs per side,
trunk first. Trunk median 2.161 s, head median 2.167 s (+0.3%, within
the 10% rule). Raw values in `perf-raw.txt`.

Base `de31dc6`, head `2621b71e5c77c5a9976519942e2b8d5b2044ff29`.

No merge; stopping at merge-ready.
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.

Statement lifecycle and getMoreResults argument handling differ

1 participant