Repository navigation
fix(jdbc): close metadata statements with their result sets - #91
Merged
Merged
Conversation
magnusp
added a commit
that referenced
this pull request
Oct 22, 2026
Closes #50 Part of #54. ## What changed `LibsqlConnection.getMetaData()` now creates `LibsqlDatabaseMetaData` on first use and returns the same instance for the lifetime of the connection. Separate connections get separate instances. A closed connection still throws. The method Javadoc documents the caching contract. The behavior test `LibsqlConnectionTest#getMetaDataIsCachedPerConnection` pins both halves: repeated calls on one connection return the same object, and another connection returns a different object. ## Contract decision Parity with xerial sqlite-jdbc 3.53.4.0. `SQLiteConnection.getSQLiteDatabaseMetaData()` caches its metadata object in a `meta` field and `getMetaData()` delegates to it, so the same instance is returned per connection. This change matches that behavior. No Hrana or sqld constraint makes parity impossible. ## Evidence Failing before the fix: ``` ./mvnw -q -pl libsql-jdbc -am -Dtest=LibsqlConnectionTest#getMetaDataIsCachedPerConnection -Dsurefire.failIfNoSpecifiedTests=false test ``` ``` [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 java.lang.AssertionError: Expecting actual: com.github.magnusp.libsql.jdbc.LibsqlDatabaseMetaData@58c34bb3 and: com.github.magnusp.libsql.jdbc.LibsqlDatabaseMetaData@56a4479a to refer to the same object ``` Passing after the fix: ``` ./mvnw -q -pl libsql-jdbc -am -Dtest=LibsqlConnectionTest#getMetaDataIsCachedPerConnection -Dsurefire.failIfNoSpecifiedTests=false test ``` No failures (exit 0). Full module suite after the fix: ``` ./mvnw -pl libsql-jdbc -am test ``` ``` Tests run: 170, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` Formatting: ``` ./mvnw spotless:apply BUILD SUCCESS ``` `libsql-itests` and `libsql-conformance` were not run: the #50 Steering evidence asks for `./mvnw -pl libsql-jdbc -am test` and `./mvnw spotless:apply`, and this change touches no live-sqld or conformance path. ## Deviations None. ## Defect coverage This is defect (4) of the four sequenced defects under #50: `getMetaData` caching. It does not reopen the earlier three defects, which were delivered in #90 (`getTableTypes`), #91 (statement leaks), and #93 (`getIndexInfo` escaping). Files touched: - `libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlConnection.java` - `libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlConnectionTest.java`
This was referenced Oct 22, 2026
Closed
magnusp
added a commit
that referenced
this pull request
Oct 22, 2026
) (#127) ## Summary Metadata result sets now close their statement through `closeStatementOnClose` instead of relying on `closeOnCompletion`. `executeMetadataQuery` no longer sets `closeOnCompletion`. It builds the result set with the flag set and attaches it to the statement. `getColumns` uses the same path, so every metadata result set exposes the statement that produced it and releases it when the caller closes the result set. Contract basis. Hrana 3 is the source of truth for transport, values, and metadata, `java.sql` is the source of truth for the JDBC surface, and xerial/sqlite-jdbc is a behavioral reference only. This change is protocol-neutral. Metadata rows still come from `execute` responses, and JDBC owns the statement and result-set close contract. ## Why #91 closed metadata statements only because `closeOnCompletion` auto-closes when the result set closes. `LibsqlResultSet.closeStatementOnClose` already existed but no caller set it, so it was dead code. A metadata result set closed without that flag leaks its statement, and only `MetadataLeakTest` would notice, which exercises `getTables` alone. #86 is resolved with the JDBC-first contract, so `closeOnCompletion` keeps its JDBC auto-close and is not changed to xerial's track-only behavior. This PR is about explicit statement ownership in the metadata code, not about defending against a #86 change that will not happen. The explicit flag is still the right contract because metadata code should not depend on a flag another class may set. ## Metadata methods covered Each method below has its own leak-count case in `LibsqlDatabaseMetaDataTest`. Every case asserts that the result set exposes a `LibsqlStatement`, that `isCloseOnCompletion()` is false, and that closing the result set closes the statement. `getTables`, `getColumns`, `getPrimaryKeys`, `getTableTypes`, `getSchemas`, `getSchemas(catalog, schemaPattern)`, `getCatalogs`, `getImportedKeys`, `getExportedKeys`, `getCrossReference`, `getTypeInfo`, `getIndexInfo`. `LibsqlDatabaseMetaData` now has one result-set construction site, `new LibsqlResultSet(statement, result, originSql, true)`, and all twelve methods reach it through `executeMetadataQuery`. ## Red and green Red at base `766e4c5` with only the new test file applied. Each method fails for the intended reason, eleven on `must not rely on closeOnCompletion` and `getColumns` on `must expose the statement that produced it`. ```text [ERROR] Tests run: 23, Failures: 12, Errors: 0, Skipped: 0, Time elapsed: 0.126 s <<< FAILURE! -- in com.github.magnusp.libsql.jdbc.LibsqlDatabaseMetaDataTest [ERROR] LibsqlDatabaseMetaDataTest.closesStatementWithoutCloseOnCompletion:159 [getTables must not rely on closeOnCompletion to close its statement] [ERROR] LibsqlDatabaseMetaDataTest.closesStatementWithoutCloseOnCompletion:155 [getColumns must expose the statement that produced it] [ERROR] LibsqlDatabaseMetaDataTest.closesStatementWithoutCloseOnCompletion:159 [getPrimaryKeys must not rely on closeOnCompletion to close its statement] [ERROR] LibsqlDatabaseMetaDataTest.closesStatementWithoutCloseOnCompletion:159 [getTableTypes must not rely on closeOnCompletion to close its statement] [ERROR] LibsqlDatabaseMetaDataTest.closesStatementWithoutCloseOnCompletion:159 [getSchemas must not rely on closeOnCompletion to close its statement] [ERROR] LibsqlDatabaseMetaDataTest.closesStatementWithoutCloseOnCompletion:159 [getSchemas(catalog, schemaPattern) must not rely on closeOnCompletion to close its statement] [ERROR] LibsqlDatabaseMetaDataTest.closesStatementWithoutCloseOnCompletion:159 [getCatalogs must not rely on closeOnCompletion to close its statement] [ERROR] LibsqlDatabaseMetaDataTest.closesStatementWithoutCloseOnCompletion:159 [getTypeInfo must not rely on closeOnCompletion to close its statement] [ERROR] LibsqlDatabaseMetaDataTest.closesStatementWithoutCloseOnCompletion:159 [getImportedKeys must not rely on closeOnCompletion to close its statement] [ERROR] LibsqlDatabaseMetaDataTest.closesStatementWithoutCloseOnCompletion:159 [getExportedKeys must not rely on closeOnCompletion to close its statement] [ERROR] LibsqlDatabaseMetaDataTest.closesStatementWithoutCloseOnCompletion:159 [getCrossReference must not rely on closeOnCompletion to close its statement] [ERROR] LibsqlDatabaseMetaDataTest.closesStatementWithoutCloseOnCompletion:159 [getIndexInfo must not rely on closeOnCompletion to close its statement] ``` Green at head `d95594b`. ```text [INFO] Tests run: 23, Failures: 0, Errors: 0, Skipped: 0 -- in com.github.magnusp.libsql.jdbc.LibsqlDatabaseMetaDataTest ``` ## Verification Base `766e4c5`, head `d95594b4d90b8e41b6c0c449f48ba4f7234ad24a`. | Lane | Command | Result | | --- | --- | --- | | 1. Regression | `./mvnw -pl libsql-jdbc test -Dtest=LibsqlDatabaseMetaDataTest` at base and head | Base 12 failures, head 23 pass | | 2. Vendored leak test | `./mvnw -pl libsql-conformance -am verify -Dit.test=MetadataLeakTest` | 1 pass, no reliance on `closeOnCompletion` | | 3. Flag off | `./mvnw -pl libsql-jdbc test -Dtest='LibsqlDatabaseMetaDataTest#closesStatementWithoutCloseOnCompletion'` | 12 pass; `isCloseOnCompletion()` is false and counts return to baseline | | 4. Module suite | `./mvnw -pl libsql-jdbc test` | 221 pass, BUILD SUCCESS | | 5. Conformance | `./mvnw -pl libsql-conformance -am verify` | 115 pass, 1 skipped, BUILD SUCCESS, no new divergence | | 6. xerial control | `./mvnw -pl libsql-conformance -am -Pxerial-control verify` | 100 pass, 1 skipped, BUILD SUCCESS, no differences to record | | 7. Format | `./mvnw spotless:check` | BUILD SUCCESS, no violations | | 8. Reactor | `./mvnw test` | 221 pass, BUILD SUCCESS | | 9. Construction sites | `grep -n "new LibsqlResultSet" LibsqlDatabaseMetaData.java` | One site, flag true; zero `closeOnCompletion` references | | 10. Package | `./mvnw -pl libsql-jdbc -am package -DskipTests` | BUILD SUCCESS | | Perf | `./mvnw -pl libsql-jdbc test` three times per side, interleaved | Trunk median 2148 ms, head median 2163 ms, +0.7 percent, within the 10 percent rule | ## Divergences and scope No protocol divergence. The protocol has no statement lifecycle field, so JDBC defines it. The product-version statement in `resolveDatabaseProductVersion` still opens a statement and closes only its result set. The queue plan scopes that fix to #92, so it is not part of this PR and has no leak-count case here. `getColumns` previously built its result set with no statement, so `getStatement()` returned null. It now has a statement like the other metadata methods. Its result set still passes a null origin SQL because the column labels are synthesized and must not drive column-origin resolution. ## Files `libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlDatabaseMetaData.java`, `libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlDatabaseMetaDataTest.java`. Fixes #119
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.
Part of #50
What changed
getSchemas(both overloads),getCatalogs,getTypeInfo,getImportedKeys,getExportedKeys,getCrossReference, andgetIndexInfonow run through the existingexecuteMetadataQueryhelper instead of creating an untracked statement directly. The helper callscloseOnCompletion(), so closing the returnedResultSetcloses its producingStatement; it also closes the statement when execution fails.Added
LibsqlDatabaseMetaDataTest.eachMetadataMethodClosesTheStatementThatProducedItsResultSet, which wraps every listed method, closes each result set, and counts statements still open afterwards.Contract decision
Parity with xerial 3.53.4.0. The vendored
MetadataLeakTestpins the same behaviour forgetTables: a metadata result set closes the statement that produced it. This PR extends that contract to every leaking method listed in #50. No behaviour deviation.Evidence
Failing before the fix:
Passing after the fix:
Formatting:
Deviations
None.
Defects covered
This PR covers defect (2) only: statement leaks in the metadata methods listed in #50. It does not change
getIndexInfotable-name escaping (defect 3) orgetMetaDatacaching (defect 4).Out-of-scope observation
resolveDatabaseProductVersionusesconnection.createStatement().executeQuery(...)in a try-with-resources block. That closes the result set but leaves the statement open becausecloseOnCompletionis not set. Not fixed here because #50 lists specific methods for this defect.