Skip to content

refactor(jdbc): unify statement result attachment - #233

Merged
magnusp merged 1 commit into
mainfrom
refactor/i211-statement-result-attachment
Oct 24, 2026
Merged

magnusp merged 1 commit into
mainfrom
refactor/i211-statement-result-attachment

Conversation

@magnusp

@magnusp magnusp commented Oct 24, 2026

Copy link
Copy Markdown
Owner

Fixes #211

What changed

  • One attach path. attachResult(resultSet, result, sql, requested) now owns every transition that makes a result set current. assignResultSet (the metadata seam) and executeResult both delegate to it, so the two near-copies cannot drift apart.
  • One discard path. discardCurrentResult(resultSet, updateCountWhenNoResultSet) is the closed branch: it marks the discarded result set closed, clears currentResultSet/resultSetRequested, restores the update count, and clears generated keys.
  • LibsqlResultSet.closeDetached() closes a result set without calling back into the statement. The statement calls it for the current result, discarded results, and generated keys.
  • closeResultSet deleted. The closed = false; closing = true; ... restore flip is gone. closed is now written only by close(), and LibsqlResultSet.closed only by close()/closeDetached(). No method temporarily reopens the statement.
  • newQueryResult(sql, result) is shared by LibsqlStatement.executeQuery and LibsqlPreparedStatement.executeQuery.
  • JdbcEscapes now holds the escape-sequence scan that checkEscapeProcessing inlined. Pure move; it exists because the unification alone did not bring LibsqlStatement under the 650-line budget.

No message, flag semantic, or lock order changed. All state transitions still use the single stateLock; no lock was added, removed, or reordered.

Preserved contract (pinned by LibsqlStatementLifecycleTest and LibsqlStatementConcurrencyTest)

  • Post-close execute, executeQuery, executeUpdate, and getConnection keep working, and a result set produced after close is returned closed.
  • A second result set request throws ResultSet already requested.
  • closeOnCompletion / isCloseOnCompletion keep the lower-case statement is closed message.
  • Metadata result sets never use close-on-completion.
  • updateCount is -1 when a result set exists and the affected row count otherwise.
  • No test or pom.xml was edited.

Diff

 .../jdbc/JdbcEscapes.java                          |  71 ++++++++++
 .../jdbc/LibsqlPreparedStatement.java              |   2 +-
 .../jdbc/LibsqlResultSet.java                      |  11 +-
 .../jdbc/LibsqlStatement.java                      | 154 +++++++---------------
 4 files changed, 126 insertions(+), 112 deletions(-)

Line counts: LibsqlStatement.java 702 -> 636 (< 650). LibsqlResultSet.java 809 -> 818. JdbcEscapes.java 71 (new).

Probe

Probe: set resultSetRequested = false instead of requested in attachResult (breaks the requested-result error), reverted.

[ERROR] Tests run: 21, Failures: 2, Errors: 0, Skipped: 0
[ERROR] LibsqlStatementLifecycleTest.getResultSetAfterExecuteQueryThrowsResultSetAlreadyRequested:34
[ERROR] LibsqlStatementLifecycleTest.preparedGetResultSetAfterExecuteQueryThrowsResultSetAlreadyRequested
[ERROR] BUILD FAILURE

After revert, LibsqlStatementLifecycleTest is 21/21 green and the full suite is green.

Lanes (final tree)

Lane Command Result
Unit ./mvnw -pl libsql-jdbc -am test 398 run, 0 failures, 0 errors
Spotless ./mvnw spotless:check BUILD SUCCESS
Conformance ./mvnw -pl libsql-conformance -am verify 272 run, 0 failures, 7 skipped
Integration DOCKER_CONFIG=... ./mvnw -pl libsql-itests -am verify 43 run, 0 failures, 0 errors

Attach the current result through one `attachResult` transition shared by
`assignResultSet` and `executeResult`, and discard it through
`discardCurrentResult` when close won the race. The closed branch now closes
the discarded result set with the new `LibsqlResultSet.closeDetached`, so
`closeResultSet` no longer flips `closed`/`closing` to let a nested close
through. `newQueryResult` is shared by both `executeQuery` paths.

`JdbcEscapes` holds the JDBC escape scan that `checkEscapeProcessing` used to
inline; the move keeps `LibsqlStatement` under 650 lines.
@magnusp
magnusp merged commit d59adf9 into main Oct 24, 2026
6 checks passed
@magnusp
magnusp deleted the refactor/i211-statement-result-attachment branch October 24, 2026 18:12
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.

Unify statement result attachment and remove the closed=false window

1 participant