Skip to content

refactor(jdbc): share one SqlTokenizer across metadata parsers - #154

Merged
magnusp merged 1 commit into
mainfrom
fix/i111-sql-tokenizer
Oct 23, 2026
Merged

magnusp merged 1 commit into
mainfrom
fix/i111-sql-tokenizer

Conversation

@magnusp

@magnusp magnusp commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Fixes #111

Contract. Hrana 3 is the source of truth for transport, values, metadata, batching, transactions, and error reporting; the parser here only decides JDBC origin and nullability, which the protocol does not supply. java.sql is the source of truth for the JDBC surface. xerial/sqlite-jdbc is the behavioral reference for which columns are auto-increment, and this PR preserves that behavior rather than changing it.

Change

One SqlTokenizer now owns tokenization for both local SQL scanners:

  • Token carries a type (WORD, IDENT, LITERAL, SYMBOL), the enclosing parenthesis depth, and a LiteralKind (NONE, STRING, BLOB), as the plan requires.
  • The shared API holds the predicate and scan helpers both call sites need: tokenize, matchingParen, identifiers, indexOfWord, indexOfAnyWord, indexOfSymbol, isWordAt, isSymbolAt, wordAt, identifierAt, and upper.
  • SqlTableReference uses it from the table-reference parser and view resolution. The duplicate matchingParen/matchingParenIn, identifiers/identifiersIn, and indexOfWord/indexOfWordIn pairs are deleted, and so are its tokenize, scanQuoted, unescape, isWordChar, Type, Token, and UNQUOTED_IDENTIFIER.
  • LibsqlResultSetMetaData uses it from the CREATE TABLE scanner. Its TokenType, Token, tokenize, skipQuoted, unescape, isWordChar, isWord, isSymbol, and identifier copies are deleted.

SqlTableReference.java goes from 1284 to 998 lines.

Red and green

The plan's Build box says behavior-preserving with no parser semantics change, and its Verify, unit box says no new behavior tests. So there is no meaningful red case: a refactor keeps every existing assertion green on both sides.

Trunk fdf3a0c and head fc663a0 both report the same parser and metadata suites:

SqlTableReferenceTest          Tests run: 34, Failures: 0, Errors: 0, Skipped: 0
LibsqlResultSetMetaDataTest    Tests run: 34, Failures: 0, Errors: 0, Skipped: 0
Total: 68, BUILD SUCCESS

Green is the equality gate: the full-module surefire case list is identical at both SHAs (307 cases, lane 9 diff empty).

Lanes (all at head fc663a0, base fdf3a0c)

Lane Command Result Log
1 ./mvnw -pl libsql-jdbc test -Dtest='SqlTableReferenceTest,LibsqlResultSetMetaDataTest' at trunk and head both 68/68; no red case exists for a pure refactor /tmp/xerial-parity-i111/lane-1-regression.log
2 grep tokenize|skipQuoted|isWordChar under libsql-jdbc/src/main not met: two other scanners remain, outside the plan's Files block (see Divergences) /tmp/xerial-parity-i111/lane-2-specific.log
3 ./mvnw -pl libsql-jdbc test 307 tests, BUILD SUCCESS /tmp/xerial-parity-i111/lane-3-specific.log
4 ./mvnw -pl libsql-jdbc test identical command to lane 3, 307 tests, BUILD SUCCESS /tmp/xerial-parity-i111/lane-4-module-suite.log
5 ./mvnw -pl libsql-conformance -am verify 123 tests, 1 skipped, BUILD SUCCESS, manifest unchanged so no new divergence line /tmp/xerial-parity-i111/lane-5-conformance.log
6 ./mvnw -pl libsql-conformance -am -Pxerial-control verify 100 tests, 1 skipped, BUILD SUCCESS /tmp/xerial-parity-i111/lane-6-control.log
7 ./mvnw spotless:check clean /tmp/xerial-parity-i111/lane-7-spotless.log
8 ./mvnw test BUILD SUCCESS /tmp/xerial-parity-i111/lane-8-reactor.log
9 surefire case sets at trunk and head identical, diff empty (307 cases each) /tmp/xerial-parity-i111/lane-9-specific.log
10 ./mvnw -pl libsql-jdbc -am package -DskipTests jar built /tmp/xerial-parity-i111/lane-10-package.log

libsql-itests was not run. This PR touches no transport, connection, or container behavior.

Perf

Metric: wall clock of ./mvnw -q -pl libsql-jdbc test, three runs per side, interleaved head/trunk.

Side Runs (ms) Median
trunk fdf3a0c 2439, 2472, 2509 2472
head fc663a0 4064, 2496, 2479 2496

Head median is +0.97% against the trunk baseline, inside the 10% rule. The one 4064 ms head run is the first run of the sequence and a warm-up outlier; trunk is always second in each pair, so the median is the comparable number. Log: /tmp/xerial-parity-i111/perf-times.txt.

Divergences

  1. Lane 2 is not met and this PR does not meet it. LibsqlStatement.java keeps a third skipQuoted for statement splitting, and LibsqlPreparedStatement.java keeps a tokenize/skipQuoted pair for result-set detection. Both files are outside the plan's Files block, and neither scanner is the near-identical copy Share one SQL tokenizer between SqlTableReference and LibsqlResultSetMetaData #111 names: one splits statements, the other only reads top-level keywords. Migrating them needs a separate PR and a plan change.
  2. Lane 1's red premise does not apply. The plan forbids new behavior tests for this PR. Equality of the existing suites and the full-module case list is the gate instead.
  3. Metadata tokenization now keeps string literals as LITERAL tokens where its private tokenizer dropped them, and X'..' is one BLOB literal where it used to be WORD x plus a dropped string. The CREATE TABLE scanner only inspects WORD and SYMBOL tokens at identifier and separator positions, and the full suite is unchanged.
  4. Metadata identifier checks use the parser's UNQUOTED_IDENTIFIER rule. The private copy accepted any WORD. The only rejected shapes are unquoted tokens that are not valid SQLite identifiers (leading digit, $, and similar), which the CREATE TABLE scanner never receives as a column name.
  5. The plan's file-size goal says "maintainable size" without a number. SqlTableReference.java is 998 lines, down 286 from trunk.

Move the two copies of tokenize, skipQuoted, unescape, and isWordChar
into SqlTokenizer, whose Token carries type, depth, and literal kind.
Both SqlTableReference and the LibsqlResultSetMetaData CREATE TABLE
scanner now use it, and the duplicated matchingParen, identifiers, and
indexOfWord helpers are gone. Tokenizer semantics are unchanged, so
parser output and auto-increment detection are unaffected.
@magnusp
magnusp merged commit 7c7d321 into main Oct 23, 2026
6 checks passed
@magnusp
magnusp deleted the fix/i111-sql-tokenizer branch October 23, 2026 02:46
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.

Share one SQL tokenizer between SqlTableReference and LibsqlResultSetMetaData

1 participant