Repository navigation
feat(jdbc): add xerial conformance suite and harden metadata - #28
Merged
Merged
Conversation
Add a conformance module for selected xerial/sqlite-jdbc tests, a pinned sqld harness with namespace cleanup, and a reproducible upstream export tool. Fix result-set metadata table resolution, integer type/class inference, metadata caching, statement/result-set close behavior, and metadata SQL escaping.
The metadata scanner currently treats AUTOINCREMENT inside a quoted default as an autoincrement column.
Strip quoted strings and identifiers before scanning CREATE TABLE SQL so a default value cannot mark the primary key as autoincrement.
Hold the namespace lock through the final DELETE so a concurrent create cannot be removed by an in-flight cleanup. Accept only 2xx and 405 responses as ready.
This was referenced Oct 21, 2026
magnusp
added a commit
that referenced
this pull request
Oct 21, 2026
…ry key (#56) Closes #38. Part of #54. ## What changed - `LibsqlResultSetMetaData.isAutoIncrement` no longer scans the whole `CREATE TABLE` text for the word `AUTOINCREMENT`. It now checks the definition of the matching column. - Added a reusable package-private helper, `LibsqlResultSetMetaData.isAutoIncrementColumn(String createSql, String columnName)`, so #42 can reuse the detection for `getColumns` instead of reimplementing the parsing. - The helper tokenizes the `CREATE TABLE` statement, skipping comments, string literals, and quoted identifiers, splits the table body into column and table constraint definitions, and looks for an unquoted `AUTOINCREMENT` keyword in the matching column definition or table-level `PRIMARY KEY (column AUTOINCREMENT)` constraint. - Added behavior tests for a column named `myautoincrement`, a table-level `PRIMARY KEY (id AUTOINCREMENT)` constraint, and a quoted identifier named `AUTOINCREMENT`. The existing real-AUTOINCREMENT, comment, and string-literal tests remain. ## Contract decision Parity with xerial 3.53.4.0. Probed xerial 3.53.4.0 directly for the cases below: | DDL | xerial `isAutoIncrement` | | --- | --- | | `create table t(myautoincrement integer primary key)` | `false` | | `create table t(id integer primary key autoincrement)` | `true` | | `create table t(id integer primary key /* AUTOINCREMENT */)` | `false` | | `create table t(id integer primary key, note text default 'AUTOINCREMENT')` | `false` | | `create table t(id integer, primary key(id autoincrement))` | `true` | | `create table t("AUTOINCREMENT" integer primary key)` | `false` | The fix matches all six. ## Evidence Failing before the fix (new test on `f38bd24`): ``` [ERROR] LibsqlResultSetMetaDataTest.doesNotReportAutoIncrementForColumnNamedAutoincrement:101 Expecting value to be false but was true [ERROR] Tests run: 9, Failures: 1, Errors: 0, Skipped: 0 ``` Passing after the fix: ``` ./mvnw -pl libsql-jdbc -am test [INFO] Tests run: 39, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ./mvnw spotless:check (no output; exit 0) ``` ## Deviations - Base branch: this PR targets `feat/jdbc-xerial-conformance` (PR #28) instead of `main`. The code this issue references (`isAutoIncrement` scanning `CREATE TABLE`, `SqlTableReference`) exists only on that branch; `origin/main` still hardcodes `isAutoIncrement` to `false` and has no table context. Implementing #38 directly on `main` would require editing `LibsqlResultSet.java` (Lane A) and duplicating #28. GitHub will retarget this PR to `main` when #28 merges. - No transport or sqld behavior deviation.
magnusp
added a commit
that referenced
this pull request
Oct 21, 2026
## What changed - Adds `LibsqlConformanceGuard`, which opens the suite's `jdbc:sqlite::memory:` probe through `DriverManager` and verifies: - the resolved driver reports `libSQL JDBC Driver`; - the resolved database product reports `SQLite (libSQL)`; - the live server returns a SQLite source id carrying libSQL's `alt1` source-id marker (`SELECT sqlite_source_id()`). - Wires the guard into `XerialLauncherSessionListener.launcherSessionOpened`, after the conformance shim is registered and before any upstream test can run. - Adds `LibsqlConformanceGuardTest` covering a valid libSQL connection, a foreign xerial-shaped connection (`SQLite JDBC` / `SQLite`), and a non-libSQL server source id. Failure messages name both the resolved and expected values, for example: ```text Conformance suite is not connected to libSQL: expected driver "libSQL JDBC Driver" and database product "SQLite (libSQL)", but resolved driver "SQLite JDBC" and database product "SQLite" ``` ## Contract decision No xerial parity question applies. This is harness integrity, so the guard fails closed: if any driver other than the pinned libSQL driver serves the probe URL, or the server is not libSQL, the suite aborts before the first upstream test. Checks use JDBC-visible behavior (`DatabaseMetaData` and a SQL query), not harness internals. ## Evidence Failed before the fix (test written first, guard absent): ```text [ERROR] LibsqlConformanceGuardTest.java:[25,9] cannot find symbol [ERROR] symbol: variable LibsqlConformanceGuard [ERROR] Failed to execute goal ...maven-compiler-plugin:3.16.0:testCompile ``` Passed after the fix: ```text ./mvnw -pl libsql-conformance -am verify Tests run: 106, Failures: 0, Errors: 0, Skipped: 1 BUILD SUCCESS ``` ```text ./mvnw -pl libsql-jdbc -am test Tests run: 36, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` ```text ./mvnw spotless:apply BUILD SUCCESS ``` Fail-fast check: temporarily changing the expected driver name made the same `verify` fail during `launcherSessionOpened`, before any test class ran, with the resolved/expected names above. The temporary edit was reverted and the green run above was repeated afterwards. ## Deviations The implementation itself makes no xerial-parity deviation. Process deviation: this branch is stacked on open PR #28 (`feat/jdbc-xerial-conformance`) because the conformance harness referenced by #44 does not exist on `main` yet. The PR diff is limited to the three files owned by this issue; retargeting to `main` after #28 merges should leave the same three-file diff. Closes #44
magnusp
added a commit
that referenced
this pull request
Oct 21, 2026
Closes #53. Part of #54. Stacked on #28 (`feat/jdbc-xerial-conformance`) because the conformance module, manifest, and vendored tree do not exist on `main` yet. This PR touches only lane F's vendoring and conformance-harness files; the vendored bytes are unchanged. > CI note: `pr.yml` only triggers for pull requests whose base is `main`, so the PR workflow does not run for this stacked PR. The repository's Automatic Dependency Submission workflow does run on pushes; it failed at `mvn validate` on head `663ec1e65119d9e9e8077f116ff39d36a05aba6d` and is fixed at head `0cf2ccfed31748fc79ed154306abf4a4c98bc9e3`. The full `spotless:check verify` reactor is green locally at that head; the commands and results are below. ## What changed - `tools/export-upstream` - Uses `upstream-xerial_sqlite-jdbc.manifest` by default and refuses to export when it is missing or empty. - Adds `--verify`: offline manifest/tree/checksum verification, including a check that no unlisted file is present. - Adds `--self-test`: exercises drift, unlisted, deleted, missing-manifest, and unrecorded-divergence cases on a temporary copy. - `CHECKSUMS.sha256` now records the pinned-tag hash (`# upstream ...`) separately from the current hash. - `libsql-conformance/pom.xml` - Runs `export-upstream --verify` at `generate-test-sources`, where the build already needs test-scope dependencies. Binding it to `validate` made `maven-antrun-plugin:run` resolve unpublished sibling artifacts and broke root `mvn validate`. - Deletes `target/generated-test-sources/xerial` before every copy, so non-clean builds cannot compile stale sources. - Copies only manifest paths with an Ant `includesfile` instead of the whole `src/test/java` tree. - `.github/workflows/ci.yml` and `.github/workflows/pr.yml` - Run `--verify` and `--self-test` before the Maven build. - Run root `mvn --batch-mode validate` so the aggregator stays independent of reactor artifacts. - `upstream-xerial_sqlite-jdbc.manifest` / `CHECKSUMS.sha256` - Document the divergence procedure and add pinned-tag hashes. ## Divergence mechanism A deliberate vendored edit must: 1. Keep the `# upstream <hash> ./path` line in `CHECKSUMS.sha256`. 2. Update the current hash on the following line. 3. Add `# divergence: <path> -- <non-empty reason>` to `upstream-xerial_sqlite-jdbc.manifest`. `--verify` rejects an unrecorded divergence and a rationale whose file still matches upstream. ## Contract decision Parity with xerial 3.53.4.0 at the pinned tag remains the contract. This is harness/build enforcement only; no JDBC behavior or vendored byte changes. Deviations: none. ## Evidence ### Failing before the fix One-byte change to a vendored file, baseline: ```text $ printf ' ' >> upstream-xerial_sqlite-jdbc/src/test/java/org/sqlite/Utils.java $ ./mvnw -q -o -pl libsql-conformance -am -DskipTests generate-test-sources build_exit_with_mutated_vendored_file=0 $ grep -R "CHECKSUMS\|export-upstream" .github/workflows none ``` Unlisted vendored file, baseline: ```text $ printf 'class ExtraTest {}\n' > upstream-xerial_sqlite-jdbc/src/test/java/org/sqlite/ExtraTest.java $ ./mvnw -q -o -pl libsql-conformance -am -DskipTests generate-test-sources build_exit=0 UNLISTED_FILE_COPIED=yes ``` Deleted vendored file with a stale generated source, baseline: ```text $ rm upstream-xerial_sqlite-jdbc/src/test/java/org/sqlite/TypeMapTest.java $ ./mvnw -q -o -pl libsql-conformance -am -DskipTests generate-test-sources build_exit=0 STALE_SOURCE_SURVIVES=yes ``` The new verifier also failed against the old checksum format before it was regenerated: ```text $ ./tools/export-upstream --verify error: malformed CHECKSUMS.sha256 exit=1 ``` ### CI fix at `0cf2ccfed31748fc79ed154306abf4a4c98bc9e3` Root cause: `maven-antrun-plugin:run` declares `requiresDependencyResolution=test`. The conformance module's validate-bound execution therefore forced resolution of `libsql-jdbc`, `libsql-client`, and `libsql-hrana-codec` even though root `mvn validate` does not build sibling reactor modules. Failing before (head `663ec1e65119d9e9e8077f116ff39d36a05aba6d`, with the local `~/.m2` snapshot artifacts removed so it matches a clean runner): ```text $ mvn --batch-mode validate ... [INFO] libSQL xerial Conformance Tests .................... FAILURE [ 0.058 s] [INFO] BUILD FAILURE [ERROR] Failed to execute goal on project libsql-conformance: Could not resolve dependencies for project com.github.magnusp.libsql:libsql-conformance:jar:0.1.0-SNAPSHOT [ERROR] dependency: com.github.magnusp.libsql:libsql-jdbc:jar:0.1.0-SNAPSHOT (test) [ERROR] Could not find artifact com.github.magnusp.libsql:libsql-jdbc:jar:0.1.0-SNAPSHOT [ERROR] dependency: com.github.magnusp.libsql:libsql-client:jar:0.1.0-SNAPSHOT (test) [ERROR] Could not find artifact com.github.magnusp.libsql:libsql-client:jar:0.1.0-SNAPSHOT [ERROR] dependency: com.github.magnusp.libsql:libsql-hrana-codec:jar:0.1.0-SNAPSHOT (test) [ERROR] Could not find artifact com.github.magnusp.libsql:libsql-hrana-codec:jar:0.1.0-SNAPSHOT exit=1 ``` Passing after (head `0cf2ccfed31748fc79ed154306abf4a4c98bc9e3`): ```text $ mvn --batch-mode validate ... [INFO] libSQL xerial Conformance Tests .................... SUCCESS [ 0.000 s] [INFO] BUILD SUCCESS exit=0 ``` ### Passing after the fix ```text $ ./tools/export-upstream --verify Verified upstream-xerial_sqlite-jdbc against upstream-xerial_sqlite-jdbc.manifest $ ./tools/export-upstream --self-test Self-test passed ``` One-byte drift, unlisted file, missing manifest, and recorded divergence: ```text one_byte_verify_exit=1 error: CHECKSUMS.sha256 does not match the vendored tree unlisted_verify_exit=1 error: vendored tree does not match upstream-xerial_sqlite-jdbc.manifest missing_manifest_verify_exit=1 error: manifest not found: upstream-xerial_sqlite-jdbc.manifest divergence_without_rationale_exit=1 divergence without rationale in upstream-xerial_sqlite-jdbc.manifest: src/test/java/org/sqlite/Utils.java divergence_with_rationale_exit=0 ``` Manifest-only copy, deleted vendored file, and stale-source removal: ```text # unlisted file present $ ./mvnw -q -o -pl libsql-conformance -am -DskipTests generate-test-sources build_exit=1 # vendored file deleted, manifest still lists it $ ./mvnw -q -o -pl libsql-conformance -am -DskipTests generate-test-sources deleted_file_non_clean_build_exit=1 # manifest entry, checksum entries, and vendored file all removed, run without clean $ ./mvnw -q -o -pl libsql-conformance -am -DskipTests generate-test-sources non_clean_build_exit=0 STALE_SOURCE_SURVIVES=no ``` ### Full verification at `0cf2ccfed31748fc79ed154306abf4a4c98bc9e3` ```text $ mvn --batch-mode validate BUILD SUCCESS $ ./mvnw -pl libsql-jdbc -am test Tests run: 36, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS $ ./mvnw -pl libsql-conformance -am verify Tests run: 103, Failures: 0, Errors: 0, Skipped: 1 BUILD SUCCESS $ ./mvnw spotless:apply BUILD SUCCESS $ DOCKER_CONFIG="$PWD/libsql-conformance/src/test/docker-config" ./mvnw spotless:check verify Reactor: codec, client, jdbc, itests, conformance all SUCCESS BUILD SUCCESS ``` The plain `./mvnw spotless:check verify` fails in this nono sandbox at `libsql-itests` because Testcontainers cannot read `~/.docker/config.json` (the documented AGENTS.md section 4 denial). Pointing `DOCKER_CONFIG` at the module's readable empty auth config makes the full reactor pass. The requested `./mvnw -pl libsql-conformance -am verify` passes without that override because the conformance module sets its own `docker.config.dir`. ## Deviations None.
magnusp
added a commit
that referenced
this pull request
Oct 21, 2026
Closes #45. Part of #54. ## What changed - `LibsqlResultSet.findColumn` now returns the first match for duplicate labels (case-insensitive), matching xerial, and reports unknown labels as `no such column: '<label>'`. - Column getters before `next()` read the first row instead of throwing. - Column getters after the last row return SQL NULL: strings, objects, bytes, decimals, and dates are `null`, numeric getters return `0`, and `getBoolean` returns `false`. Typed numeric `getObject(Class)` still throws `Bad value for type X`, as xerial does. - `getRow()` returns `0` before the first row, the 1-based current row while positioned, and the last row number after the end, including after repeated `next()` calls. - `isLast()` now always throws `SQLFeatureNotSupportedException("not supported by sqlite")`. - `wasNull()` now tracks the last column read and the current cursor position, including xerial's boundary exceptions (see the table below). - Added class-level Javadoc that documents the navigation contract. - Added `LibsqlResultSetNavigationTest` covering duplicate labels and every cursor boundary in the acceptance criteria. ## Contract decision Parity with xerial 3.53.4.0 at the pinned tag, as required by #54. Every claim was re-probed against the published `org.xerial:sqlite-jdbc:3.53.4.0` jar (and cross-checked with the vendored tag sources) before pinning tests. | Method | Chosen target (xerial 3.53.4.0) | | --- | --- | | `findColumn` | First duplicate label wins; case-insensitive; unknown label throws `SQLException("no such column: '<label>'")`. | | Getters before `next()` | Read the first row (xerial's synthetic row before `next()`). | | Getters after the end | SQL NULL: `null` for text/object/bytes/decimal/date getters, `0` for numeric getters, `false` for `getBoolean`; typed numeric `getObject(Class)` throws `Bad value for type X`. | | `getRow` | `0` before the first row, 1-based current row while positioned, and the last row number after the end (stays there on repeated `next()` returns of `false`). | | `isLast` | Always throws `SQLFeatureNotSupportedException("not supported by sqlite")`. | | `wasNull` | Reflects the last column read at the current cursor position. Throws `SQLException` while no column has been read in the current row: `column 0 out of bounds [1,N]` before the first `next()`, and `column -1 out of bounds [1,N]` immediately after transitioning past the last row. After a getter at the past-end position it returns `true`, because the pointer reports SQL NULL. | Probe corrections to the issue body: `wasNull` before any getter is not `false`; it throws. Only `getRow` and `getInt` had been re-verified in #28, so the duplicate-label, `isLast`, and `wasNull` claims were probed again here. ## Evidence Failing before the fix, on the current code (`./mvnw -pl libsql-jdbc -am -Dtest=LibsqlResultSetNavigationTest -Dsurefire.failIfNoSpecifiedTests=false test`): ```text [ERROR] Tests run: 10, Failures: 5, Errors: 4, Skipped: 0 [ERROR] LibsqlResultSetNavigationTest.findColumnReturnsTheFirstMatchingDuplicateLabel:40 expected: 1 but was: 2 [ERROR] LibsqlResultSetNavigationTest.getRowIsZeroBeforeTheFirstRowAndStaysOnTheLastRowAfterTheEnd:94 expected: 2 but was: 0 [ERROR] LibsqlResultSetNavigationTest.isLastAlwaysThrowsLikeXerial:112 Expecting code to raise a throwable. [ERROR] LibsqlResultSetNavigationTest.wasNullThrowsBeforeAnyGetterAndAfterTheEndWithoutAGetter:124 Expecting code to raise a throwable. [ERROR] LibsqlResultSetNavigationTest.gettersReadTheFirstRowBeforeNext:57 » SQL Cursor is not pointing to a valid row [ERROR] LibsqlResultSetNavigationTest.gettersReturnNullAndZeroAfterTheLastRow:68 » SQL Cursor is not pointing to a valid row [ERROR] LibsqlResultSetNavigationTest.gettersReturnNullAndZeroWhenTheResultSetHasNoRows:79 » SQL Cursor is not pointing to a valid row [ERROR] LibsqlResultSetNavigationTest.wasNullTracksTheLastGetterRead:140 » SQL Cursor is not pointing to a valid row ``` Passing after the fix: ```text $ ./mvnw -pl libsql-jdbc -am test [INFO] Tests run: 132, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ``` ```text $ ./mvnw -pl libsql-conformance -am verify [INFO] Tests run: 115, Failures: 0, Errors: 0, Skipped: 1 [INFO] BUILD SUCCESS ``` ```text $ ./mvnw spotless:apply $ ./mvnw spotless:check BUILD SUCCESS ``` The conformance run included the selected xerial `ResultSetTest` (21 tests) against sqld. ## Deviations None. The change is pure parity with xerial 3.53.4.0; the Hrana transport does not prevent any of these semantics. ## Out-of-scope defect noticed `isFirst()` still differs from xerial after the end of a single-row result. xerial returns `true` there because its internal row counter stays at the last row; this implementation returns `false`. That method is not one of the six navigation methods in this issue's contract and was left unchanged. Likewise `isBeforeFirst`/`isAfterLast` boundary semantics were left as-is; they already matched the probed behavior for the cases covered.
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.
Why
The JDBC driver had no regression suite against the xerial tests, and its result-set metadata missed table context and integer typing rules. This adds a pinned conformance harness and fixes the metadata behavior those tests expose.
What changed
libsql-conformancewith a pinnedlibsql-serverimage, namespace cleanup, and the selected xerial tests.LibsqlResultSetMetaData,LibsqlResultSet,LibsqlStatement, andLibsqlDatabaseMetaData.Scope
Covers the selected xerial test batch and the metadata fixes it needs. Leaves out the full upstream repository export and later test batches.
Blast Radius
Touches JDBC metadata, result-set lifecycle, and the test harness. The production changes affect callers that read column types, autoincrement flags, and table names. The conformance suite is test-only.
Verification
./mvnw -pl libsql-jdbc -am testpassed 36 tests../mvnw -pl libsql-conformance -am verifypassed 103 tests with 1 skipped.DOCKER_CONFIG=/tmp/docker-config ./mvnw spotless:check verifypassed, including 14 integration tests.Issue review corrections
Checked issues #29 through #54 on branch
feat/jdbc-xerial-conformance, against xerial/sqlite-jdbc 3.53.4.0 at commit cab7981 and the published 3.53.4.0 jar. The issue bodies were not edited.'2', not true. The body lists the libSQL results as true, true, and true, and the reproduction says'2'is true in both drivers.LibsqlResultSet.getBooleanaccepts only"true"or"1"as true text (LibsqlResultSet.java:133). A direct probe returned true, false, true for'true','2', and0.5; xerial returned false, true, false.BEGINdoes not succeed aftersetAutoCommit(false). The first statement callsensureTransactionStarted, which sendsBEGIN IMMEDIATEbefore the explicitBEGIN, so the explicitBEGINis a second begin and fails.LibsqlConnection.executeStatementcallsensureTransactionStartedatLibsqlConnection.java:76, and that method sendsBEGIN IMMEDIATEatLibsqlConnection.java:160-172.getRow()returns 0 beforenext(), not 1. After the end it returns the last row number, which is 1 for a one-row result, not 0. The reproduction comment and the sentence "xerial returns row 1 before next()" are wrong. Column getters, notgetRow, return the first row beforenext().getRow=0beforenext(),getRow=1after the end,getInt=1beforenext(),getInt=0after the end.JDBC3ResultSet.getRow()returns therowfield, initialized to 0 inCoreResultSet.java:44.getTableTypesand statement leaks. The body also covers unescaped table names ingetIndexInfoandgetMetaDatacaching.No correction was needed for #29, #30, #31, #33, #34, #35, #36, #37, #38, #39, #41, #42, #43, #44, #46, #47, #48, #49, #51, #52, #53, or #54. The #29 ranges start on
@Overridebut still cover the cited methods. The #51 count is correct. There are 39 upstream test classes, 2 of them are architecture tests, so 37 functional classes remain, and the manifest selects 10. The #49 claim holds because no lock or synchronized block exists in the client or JDBC modules. The #35 and #46 overlap is acceptable because they cover different metadata methods.Follow-up issue changes
getTableTypescompleteness, metadata statement leaks,getIndexInfotable-name escaping, and metadata caching. If it stays as one issue, retitle it to name all four.