Skip to content

Give non-streamable result sets a route of their own - #16

Merged
ShawnChen-Sirius merged 6 commits into
mainfrom
fix/issue-12-nonstreamable-results
Sep 9, 2026
Merged

ShawnChen-Sirius merged 6 commits into
mainfrom
fix/issue-12-nonstreamable-results

Conversation

@ShawnChen-Sirius

@ShawnChen-Sirius ShawnChen-Sirius commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

Closes #12.

Every row-returning statement that is not a SELECT failed, because the driver routed
everything that produces a result set to chdb_stream_query_arrow_n and the engine refuses to
stream anything that is not a real SELECT pipeline. SHOW, DESCRIBE, DESC, EXPLAIN and
EXISTS had no route at all — six of the seven statements in the issue.

chdb_query_arrow_n exports through the same Arrow C Data Interface the streaming path already
consumes, so ArrowSchemaView, ArrowBatch and ChdbResultSet are unchanged.

How the route is chosen

The issue left this open, with a warning: retrying a failed stream open is safe only for
statements that cannot have written anything, and StatementShape refuses to retry in general
for that reason.

A keyword whitelist cannot stand in for "cannot have written", and there is a counter-example:

WITH q AS (SELECT 2 AS v) INSERT INTO t SELECT v FROM q

That parses, writes, has leading keyword WITH, and is refused by the streaming door with
exactly the Streaming query is not supported text a SHOW gets. The engine does raise the
refusal before execution (ClientBase.cpp:1343, ahead of processParsedSingleQuery — confirmed
by observing the table stay empty), but that is only visible through an error-message prefix,
and keying a re-execution decision on a string whose drift silently duplicates writes is worse
than the error it avoids.

So there is no fallback. StatementShape.producesResultSet becomes a three-valued
route(int[], String) — NO_RESULT_SET / STREAMED_RESULT_SET / MATERIALIZED_RESULT_SET.
The engine's classifier still decides whether there is a result set; the keyword scan decides
which door, because CHDB_QUERY_READ_ONLY covers both a SELECT and a SHOW. The original
no-retry rule is preserved verbatim; what changed is that these statements never reach the
streaming door.

Verified

macos-aarch64, engine v26.7.0:

  • 165 unit tests, 0 failures (StatementShapeTest 51)
  • 122 integration tests, 0 failures — full suite, three times, on JDK 11, 21 and 26
  • native shim harness chdb_jni_test: 199 checks, 0 failures
  • the issue's exact statement list: 6 of 7 FAIL → all OK, with real column names and types
    (DESCRIBE 7 String columns; SHOW TABLES name:String; EXISTS TABLE result:UInt8; all
    seven EXPLAIN forms; CHECK TABLE part_path/is_passed/message)
  • leak check on the new route: 200,000 DESCRIBEs under -Xmx128m. RSS 221 MB → 238 MB by
    50k, then flat (239,008 / 239,088 / 239,120 / 239,120 / 239,168 / 239,168 KB) through 200k.
    Handle counts return to 0.

JNI ABI version 1 → 2, since Java declares a native method an older shim does not export.

Two things the issue asked for that turned out not to exist

  • TABLE t shorthand is not a statement this engine has. v26.7.0 parses TABLE plain as
    SELECT TABLE AS plain (error 47) and TABLE db.d does not parse at all (error 62). Asserted
    in tableShorthandIsNotSupportedByTheEngine, including that neither failure is a streaming
    refusal. VALUES (1),(2) is the same story.
  • No server-side parameters on the materialised route. chdb_query_arrow_with_settings_n
    takes a param map but lives in namespace CHDB and is absent from both
    libchdb_export_macos.txt and libchdb_export.map. Reported as SQLSTATE 0A000 with a
    workaround, and recorded as an upstream ask.

linux-x86_64, linux-aarch64 and macos-x86_64 are unverified — no hardware here, CI only.

🤖 Generated with Claude Code

Note

Add materialized Arrow execution route for non-streamable result sets

  • Splits statement classification into three routes: streamed results (SELECT, WITH, VALUES, TABLE), materialized results (SHOW, DESCRIBE, EXPLAIN, EXISTS, CHECK), and no-result execution. Each statement is routed once before execution with no retry on failure.
  • Adds a native JNI entry point ChdbNative.streamOpenMaterialized that runs a query to completion via chdb_query_arrow_n and exposes the exported ArrowArrayStream for batch iteration. Parameterized statements on this route are rejected with SQLSTATE 0A000 because no parameterized materialized entry point exists.
  • Reworks timeout handling to use an execution counter under timeoutLock; stale timeout callbacks from prior executions are rejected instead of cancelling the current one. A post-open guard closes the result set if a timeout or cancellation occurred during the uninterruptible open window.
  • Fixes nested ClickHouse block comment scanning in StatementShape.leadingKeyword so depth-counted comments are skipped correctly.
  • Risk: Materialized routes execute fully server-side before the first batch is read, so JDBC row limits are ineffective; max_memory_usage is the only guard against OOM. JNI ABI version bump from 1 to 2 requires matching native and Java sides at load time.

Macroscope summarized 646719f.

Comment thread chdb-jni/src/chdb_jni.cpp
Comment thread chdb-jdbc/src/main/java/org/chdb/jdbc/ChdbStatement.java
ShawnChen-Sirius and others added 4 commits September 9, 2026 20:13
Every row-returning statement that is not a SELECT failed. chdb_stream_query_arrow_n
admits a statement only if it parses as an ASTSelectWithUnionQuery, so SHOW, DESCRIBE,
DESC, EXPLAIN, EXISTS and CHECK all came back as "Streaming query is not supported" --
which is what a JDBC GUI issues on connect, and what a person types first.

chdb_query_arrow_n accepts all of them and exports the whole result through the same
Arrow C Data Interface the streaming path already consumes, so ArrowSchemaView,
ArrowBatch and ChdbResultSet are unchanged. The shim gets a second open entry point that
registers the exported stream as an ordinary stream handle; fetching branches on the
handle's batch source, and nothing above the shim can tell the two apart.

Routing is decided once, before execution, by a third keyword state -- not by retrying a
refused stream open. A retry needs a proof that the first attempt wrote nothing, and
there is none to be had: WITH q AS (SELECT 2 AS v) INSERT INTO t SELECT v FROM q parses,
writes, and is refused by the streaming door with exactly the text a SHOW gets, so "the
leading keyword introduces a result set" cannot stand in for "this cannot have written".
The engine does raise that refusal before executing anything, but keying a re-execution
decision on an error-message prefix means the day upstream reworks that path the driver
silently starts duplicating writes. StatementShape's original rule therefore stands: a
statement the engine refuses is reported, never re-run.

Two limits come with the new route, both documented rather than papered over: the result
is materialized rather than bounded by one batch, which is safe only because these
statements return schema-sized answers; and the engine exports no parameter-binding form
of chdb_query_arrow_n, so a PreparedStatement with a ? in a SHOW is refused instead of
having its values interpolated into the SQL.

The JNI ABI version goes to 2: Java now declares a native method an older shim does not
export, which is a load-time mismatch the version check exists to catch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every cancel the chDB C ABI exports takes a result or stream handle --
chdb_stream_cancel_query, chdb_streaming_cancel_query, chdb_stream_cancel_insert -- and
there is no connection-level cancel. So the call that produces the handle cannot be
interrupted: a timer firing inside it finds inFlightStream still null and returns having
done nothing, and executeQuery then came back successfully, with a usable result set,
after the deadline the caller set.

Measured on v26.7.0: setQueryTimeout(1) on
SELECT max(sipHash64(number)) FROM numbers(2000000000) returned a working result set
after 12.65 seconds.

This is not specific to the materialized route, though that is where review found it.
The window is the open, and how much of the statement lives there depends on the
statement: milliseconds for a SELECT that emits as it scans, the whole query for a full
aggregate or an ORDER BY without a LIMIT, and the whole statement for anything on
chdb_query_arrow_n. The fix is therefore at the shared boundary rather than on one route:
once the open returns, an expired timeout closes the result set and raises
SQLTimeoutException, and a cancel that arrived in the same window raises 57014. The
mid-stream cancel path is untouched -- it still works through inFlightStream, and both
existing cancellation tests pass unchanged.

It is a deadline, not an interrupt: the statement has already run and the work is spent
either way. But the caller is told, rather than handed a result that arrived after the
time it allowed, and setQueryTimeout stops being a setting that silently does nothing.

Also documents what review was right to ask about on memory, having measured it. The
materialized route's peak is entirely engine-side and already spent before the driver
holds a handle: RSS after the open, with not one row read, is +234 MB for a 108 MB result
and +896 MB for an 864 MB one, and does not move while the caller iterates, against
48 KB-6.5 MB for the streaming route on the same queries. So no driver-side cap can help;
what does bound it is SET max_memory_usage, which covers this path and turns an oversized
materialization into a clean Code 241. Two things fell out of measuring that: no setting
passed in the connect argument vector reaches the session at all, and errors on this path
arrive wrapped two or three times.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…oes not

The finding recorded with the materialised route said settings in the connect
argument vector never reach the session, without naming a version, and
docs/memory.md went on recommending ?max_memory_usage=... as the way to cap the
engine in a container. On the pinned v26.7.0 engine that URL is silently inert:
measured through JDBC with max_threads=7, max_result_rows=13 and
max_block_size=4096, the session reports auto(18), 0 and 65409, while the same
values applied with SET all take effect.

It is not our argument vector. It has the same --key=value shape chdb-core's own
Python binding builds in LocalChdb.cpp. EmbeddedServer is a process-wide
singleton that reads argv only when the first connection boots it, so
per-connection settings were swallowed into its config layer -- chdb-core #191,
fixed by 3231c03afca on 2026-08-24 with ChdbClient::applyCmdSettings. That is
twelve days after the v26.7.0 tag, which is why this repository sees it and a
binding built on a later engine does not.

Measured on v26.7.2-rc.2, the same URL yields 7, 13 and 4096, so the promise in
ChdbUrl.toConnectArguments holds from that release onwards and resolves with the
engine bump in issue #8 rather than needing anything here. Both documents now
name the version boundary, and memory.md tells a reader on the current baseline
to use SET instead of a URL property.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The baseline now exports chdb_classify_query_n, and the obvious next step is to ask the
engine which door a statement goes through instead of scanning its first keyword. It does
not work, and that is worth writing down rather than rediscovering.

Measured on v26.7.2-rc.2, the classifier returns CHDB_QUERY_READ_ONLY for SELECT and for
every statement issue #12 is about -- SHOW, SHOW CREATE TABLE, DESCRIBE, DESC, all seven
EXPLAIN forms, EXISTS and CHECK TABLE -- with no other field separating them. It divides
read from write, not streamable from not, so routing on it alone would put every SHOW
back on the streaming door, which is the bug. The keyword scan is therefore the long-term
mechanism for that half rather than a stopgap, and stays until the ABI can say "this
statement has a result set the streaming path will accept".

It does close one gap. WITH q AS (...) INSERT INTO t SELECT ... comes back MUTATING, so it
now routes to chdb_query_n and executes correctly instead of hitting the streaming
refusal. The negative test asserted the old engine's error text, which made it fail on the
new baseline for the right reason; it now asserts the property instead -- a call that
throws moves the row count by 0 and a call that succeeds by exactly 1, never 2 -- which
holds whichever engine is pinned and is stronger for the case that does run.

Re-measuring the rest against the new baseline turned up something load-bearing that the
old one had hidden. Opening a streaming query and closing it without cancelling leaves the
engine in streaming mode for that connection, and the next chdb_query_arrow_n on it fails
with the streaming refusal even though it went through the materialised entry point;
cancelling or draining first clears it. The driver is safe only because
ChdbResultSet.close() already cancels an unexhausted stream, for the unrelated reason of
not making the engine finish a query nobody is reading. "Read one row of a huge result and
close" is the documented cheap case and a SHOW TABLES right after it is what a JDBC GUI
does, so that dependency now has a test.

Also corrects what the previous commit said about settings in a JDBC URL. The baseline
moved while that was being written, so the recommendation is a URL property again:
measured with ?max_threads=7&max_result_rows=13&max_block_size=4096, system.settings
reports 7, 13 and 4096 with changed=1, and max_result_rows is enforced rather than merely
readable -- a 100-row SELECT fails with code 396. The v26.7.0 behaviour is kept as a note
for anyone pointing the loader at an older library, not as the advice.

Two smaller things measured on the way, both recorded and neither actionable here: the
streaming refusal is unchanged on the new baseline for all fourteen statement forms, and
chdb_query_arrow_n still executes a statement with no result header before reporting
"Missing result header for Arrow output" -- handed an INSERT it writes the row and then
returns an error, which is the concrete reason a retry-on-the-materialised-door fallback
would have been unsafe rather than merely unprovable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ShawnChen-Sirius
ShawnChen-Sirius force-pushed the fix/issue-12-nonstreamable-results branch from 4d23d87 to d670b67 Compare September 9, 2026 08:25
Comment thread docs/unsupported.md Outdated
Comment thread chdb-jni/src/chdb_jni.cpp Outdated
Comment thread chdb-jdbc/src/main/java/org/chdb/jdbc/StatementShape.java
Comment thread chdb-jni/src/chdb_jni.cpp Outdated
Comment thread docs/memory.md Outdated
…fects

Five review findings, four of them real.

The documented workaround for "this route takes no parameters" was to paste the value
into a plain Statement -- the injection that server-side binding exists to prevent, two
sentences after saying so. It is not theoretical: measured on v26.7.2-rc.2, a LIKE pattern
of `%' LIMIT 1 -- ` interpolated into SHOW TABLES silently drops the result from three rows
to one, and `%' INTO OUTFILE '/tmp/x.tsv' TRUNCATE -- ` is accepted through execute() and
writes the file. The advice is now the system tables and only the system tables, with the
seven substitutions spelled out and each one verified to work with bound parameters. EXPLAIN
is the one form with no system.* equivalent, and the document says what that does and does
not license. The runtime message in ChdbStatement said the same wrong thing and now says
the same right thing.

Block comments nest in ClickHouse. leadingKeyword stopped at the first close marker, so
`/* /* */ */ SELECT ...` read no keyword at all and `/* a /* b */ c */ SELECT ...` read
`c` -- and with the classifier reporting the statement READ_ONLY, that sent a streamable
SELECT to the materialized door: +496 MB of RSS for a four-million-row result against
+2 MB for the same query behind a non-nested comment. The scan now counts depth, and its
verdict was checked against the engine's parser on nine comment shapes -- balanced,
unbalanced, unterminated and stray-close -- which agree in every case, including that
`/* /* */ SELECT 1` really does swallow the statement.

That fix raised the question of which way an unreadable prefix should err, and the answer
changed: it now goes to the streaming door rather than the materialized one. The
materialized door executes what it is given before checking there is a result header to
export, so handed a write it writes the row and then reports an error -- it is the one
guess that can change data. The streaming door refuses anything that is not a SELECT
pipeline before executing it, and its mistake costs an error rather than an unbounded
buffer.

streamCancel and the fetch path read ConnHandle::conn under the stream's lock while
closeConnection wrote it under the connection's. Pre-existing on both paths -- origin/main
has the same unsynchronised read, and chdb_close_conn frees what the pointer points at, so
it was a use-after-free waiting on a thread interleaving rather than only a data race on a
non-atomic member. Both now take the connection lock inside the stream's and hold it across
the engine call. The order is StreamHandle::mutex -> ConnHandle::mutex ->
HandleRegistry::mutex_, written down in chdb_jni_handles.h with why nothing goes the other
way, and primeAndRegister drops the lock it used to take so the now-self-locking fetch
cannot recurse on it.

The materialized open keyed its cleanup on a flag set after the engine call returned, so a
failure that left a release callback behind would leak it. Cleanup is now keyed on the
callback itself, which is the thing that says whether there is anything to release, and
has_array_stream is gone rather than left as a second source of truth. On this engine the
old code did not actually leak -- chdb-core zeroes the out_stream before every path that
can fail with our arguments -- so this is the invariant being wrong rather than an
observable leak.

Also finishes a correction from the previous commit: docs/memory.md said "Set it with SET,
not in the JDBC URL" in a second place that the earlier fix missed. Both now say the same
thing, which is what the current baseline does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread chdb-jdbc/src/main/java/org/chdb/jdbc/ChdbStatement.java
QueryTimeout.stop() is Future.cancel(false), so a task the scheduler has already begun
runs to completion and stop() does not wait for it. The task's whole body was
statement.cancel(), which sets a cancel flag shared across executions and cancels whatever
stream is in flight now -- so a timer armed for one query could land on the next query on
the same Statement.

The root of that is older than this branch: origin/main has the same cancel(false), the
same bare statement.cancel() in the task, and the same shared flag. What this branch added
was a second and far more visible symptom. checkDeadlineSurvivedTheOpen fails an execution
whose cancel flag is set, so a stale timer no longer had to win a race against a mid-stream
fetch to be noticed -- it was enough to set the flag, and a perfectly valid statement came
back as SQLSTATE 57014. Intermittent, interleaving-dependent, and on a pooled connection
running thousands of statements it reads as an unstable engine rather than as a driver bug.

Executions are now numbered. The scheduled task carries the number it was armed for and
hands it back to ChdbStatement.queryTimeoutFired, which drops the delivery unless that
execution is still the one running. The check and the cancel are both under the lock
executeInternal takes to claim its number, because a compare that is not held across the
act it guards is the same bug in a smaller window: the timer could pass the check and then
be descheduled while the next execution starts.

Statement.cancel() deliberately does not go through that gate. An application calling it
means "stop whatever is running now", which is never stale, and gating it would have
quietly turned it into a no-op -- so the timeout path got its own entry point rather than
the check being pushed down into cancel().

Two smaller things found looking for others of the same shape. activeTimeout is now
volatile and reset at the start of every execution, so timedOut() cannot report a deadline
belonging to a statement already finished when a result set is read on another thread. And
close() disarms the timeout directly rather than only through the result set, so a closed
statement leaves nothing in the shared scheduler queue holding a reference to it for the
length of its timeout.

The regression test drives ChdbStatement.queryTimeoutFired directly from a test in the
same package. That is not a shortcut around the real path -- it is the real path, since the
scheduled task's body is exactly one call to that method -- so a stale delivery is
reproducible deterministically without a test-only hook in production code and without
waiting on a microsecond-wide interleaving. Three of its four cases fail with the guard
removed. That a current number still fires is covered by the existing wall-clock timeout
tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ShawnChen-Sirius
ShawnChen-Sirius merged commit 5c666ab into main Sep 9, 2026
30 checks passed
ShawnChen-Sirius added a commit that referenced this pull request Sep 9, 2026
PR #16 landed a second native statement-start entry point while this
branch was in flight: chdb_query_arrow_n via streamOpenMaterialized,
for the SHOW / DESCRIBE / EXPLAIN / EXISTS / CHECK route. It carries
the same hazard as the streaming open, measured rather than assumed --
DESC over a 1.9 GB JSONEachRow file with the schema-inference row
limit raised, which is the only statement on that route that reads
data rather than metadata, two threads, main returning while they are
in flight:

  #16 merged, before this branch, hook on    6 aborts in 40
  #16 merged, before this branch, hook off   0 aborts in 20
  with this branch, hook on                  0 aborts in 80

So the abort is about closing the connection, not about how the
statement was started. Finding a long enough statement on that route
took some looking: CHECK TABLE on a 30 M-row MergeTree is 87 ms and
EXPLAIN PLAN / EXPLAIN ESTIMATE are 1-5 ms, because everything else
there is metadata. The DESC is 25.9 s.

Nothing was needed to cover it. The gate is entered at the top of
executeInternal's try, before the route is decided, so route()'s
classifier, both opens inside openStream() and chdb_query_n in
runMaterialized() are all inside it -- covering the body rather than
wrapping each call is why a new entry point landed inside the gate
without anyone remembering to put it there. That is now asserted
rather than left to hold by luck:
aClaimedConnectionRefusesNewStatements tries one statement of each
route against a claimed connection and requires 08003 from all three,
so moving the gate into a branch turns red.

Semantics of the conflict resolution in executeInternal, which is the
same control flow both sides changed:

Order. The gate is entered before QueryTimeout.start(), so a refusal
cannot leave a timer armed against an execution that never ran. #16's
execution-number mechanism would probably discard such a timer, but
correctness here should not rest on another mechanism's side effect,
so the ordering makes the case impossible instead. The gate is entered
after statementSlot().acquire(), equally deliberately: a thread
waiting for another statement's result set to close is not inside the
engine, and holding the gate across that wait would block the hook for
as long as the other statement lives. #16's execution-number claim
stays where it is, ahead of both; a number claimed by a refused
execution is harmless and still invalidates any older timer.

Nesting. One try, one finally, both concerns in it -- exit() guarded
by the entered flag, slot release guarded by handedOff. That covers
the path #16 added: checkDeadlineSurvivedTheOpen() throwing 57014
after handedOff was set unwinds through the inner catch that stops the
timeout and then through this finally, which exits the gate and
correctly does not re-release a slot that closeCurrentResultSet()
already released.

Re-measured everything after the rebase; every earlier conclusion
still holds. front() aborts 0/40 on the streaming route and 0/80 on
the materialized one. Clean exit still pays nothing: shutdown phase
8/9/8/8 ms against 426-446 ms leaked. A failed connect still does not
stall a later exit: 12-14 ms. All four mutants still killed, including
the one that moves the refusal outside the slot-releasing try, which
matters most because that try was just rearranged.

Full verify green on JDK 11, 21 and 26.

Refs #22.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ShawnChen-Sirius added a commit that referenced this pull request Sep 9, 2026
PR #16 rearranged that method's try structure around the query-timeout
check, and the gate's exit shares its finally with the statement
slot's release, so the nesting is now the part most worth pinning. A
gate left raised is silent in both directions: the shutdown hook skips
that connection for the rest of the process, so it stops doing its
job, and nothing is refused either, so nothing reports it.

theGateComesBackOnEveryPath walks each exit and checks the count is
back to zero after it: a streaming success, a materialized success, a
statement with no result set, one the engine rejects, one the driver
rejects for the wrong shape, and -- the path #16 added -- a deadline
that expired during the open, where checkDeadlineSurvivedTheOpen
closes the result set and throws 57014 from a point where handedOff is
already set. It ends by claiming the connection, which is the property
all of that is in service of. The 57014 branch is the one actually
taken here rather than the fast-machine fallback, so it is covered
rather than merely allowed for.

Verified not vacuous by a fifth mutant, the plausible nesting mistake:
exiting the gate only when the slot was not handed off. That leaks it
on every successful streaming statement and on the 57014 path, and the
test reports the successful streaming statement with a count of 1.

Needs a non-terminal read of the gate, because claimForShutdownClose()
can only answer "was it free?" once per connection and this has to ask
after each of half a dozen statements. ChdbConnection.executionsInFlight()
is package-private and ShutdownHookAccess exposes it to the ITs the
same way it already exposes the claim.

Refs #22.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ShawnChen-Sirius added a commit that referenced this pull request Sep 9, 2026
shuttingDown() carried SQLSTATE 08003 on a plain
SQLNonTransientException. JDBC 4 defines SQLNonTransientConnectionException
for class 08, so code that catches the subtype instead of reading the
SQLSTATE string did not see the refusal at all. Reported on PR #23 and
correct.

One correction to the reported consequence, because it matters for
what to test: a pool is not the caller that was affected. HikariCP
evicts on getSQLState().startsWith("08") -- read out of
ProxyConnection.checkException in 5.1.0 -- so it acted on the SQLSTATE
alone and was already doing the right thing. The callers this reaches
are the ones that catch the JDBC subtype. Both are now covered:
SqlStateTypeTest for the type, and HikariPoolIT for the pool.

The same mistake was in seven other places, so this is the audit
rather than the one line. Every SQLSTATE-bearing throw site in the
driver, checked against the subtype JDBC names for its class:

  class 08 -> SQLNonTransientConnectionException
    08001  ChdbExceptions.wrap(UnsupportedPlatformException)  was SQLNonTransientException
    08001  ChdbUrl, five sites (bad URL, unresolvable path,
           malformed property, missing UTF-8, bad percent-encoding)  was SQLException
    08003  ChdbExceptions.shuttingDown()                      was SQLNonTransientException
    08004  StoragePathRegistry.acquire()                      was SQLException
  class 0A -> SQLFeatureNotSupportedException
    0A000  six unwrap() sites and #16's materialized-parameter refusal  was SQLException
           -- and docs/unsupported.md's opening line already promised this type
  class 22 -> SQLDataException
    22000, 22003, 22007, 22018, 22023  eighteen sites            was SQLException
  class 42 -> SQLSyntaxErrorException
    42601  SqlParameterLexer, three sites                     was SQLException
    42S22  ChdbResultSet column-not-found                      was SQLException
    42804  ChdbExceptions type-mismatch mapping               was SQLNonTransientException

Every one of those is a narrowing, so nothing that caught SQLException
or SQLNonTransientException stops catching it.

Checked and left alone, with the reason now written down where it
was not already:

  25000  no JDBC subtype for class 25. Also the loosest-fitting state
         in the driver -- 25000 is "invalid transaction state" and chDB
         has no transactions -- but the README, docs/unsupported.md and
         sameThreadReuseIsRefused all pin it, so changing the state
         belongs in its own change, not in a type audit.
  53200  SQLTransientException, and right: a retry after
         max_memory_usage may genuinely succeed. Class 53 has no
         subtype.
  70100  plain SQLException, not SQLTransientException. An interrupt is
         almost always a shutdown, and telling retry logic the operation
         is worth repeating is the opposite of what it asked for.
  HY010  SQLNonTransientException. Vendor space, no subtype.
  07xxx, 24000, HY000  no subtype for those classes.
  no SQLSTATE at all  the default engine-error mapping, deliberately:
         guessing a class wrongly is worse than declining to guess.

One thing did change on semantics rather than on type-matching. The
engine's QUERY_WAS_CANCELLED (394) was mapped to SQLTimeoutException,
which told a caller to retry with a longer deadline for a cancel()
somebody made on purpose -- and disagreed with the driver's own two
cancel sites, which already reported a known cancel as a plain
SQLException. The engine cannot say who asked, so it now agrees with
them. A timeout the driver armed still arrives as SQLTimeoutException,
from the sites that know it was theirs. 57014 is therefore the one
state thrown as two types, split by error code rather than by state,
and that is now stated in a comment.

SqlStateTypeTest has two halves. The first pins the types the factory
produces, including the assertThrows and the catch clause a caller
would actually write -- the thing the finding was about. The second is
a rule over the whole driver's source, which is the only half that can
see a *new* throw site: adding an error code now has to answer the
question rather than skip it. It has a floor of 40 sites so a refactor
cannot make it vacuously true, and a companion test pins the class-to-
subtype map itself so that removing an entry is a visible choice.

Verified the tests would have caught the original bug: restoring
SQLNonTransientException on shuttingDown() fails three of them --
the direct type assertion, the assertThrows, and the source rule.

HikariPoolIT.aClaimedConnectionIsEvictedFromThePool covers the end of
the chain: a claimed connection is evicted rather than handed to the
next borrower. Pool size one on purpose, so a pool that did not evict
would have to return the claimed connection and the SELECT would fail.
It passed before this change too, for the SQLSTATE reason above, and
says so.

docs/unsupported.md gains the SQLSTATE-to-type table, since which type
to catch is now a contract worth writing down.

Full verify green on JDK 11 and 26.

Refs #22.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

SHOW, DESCRIBE, EXPLAIN and EXISTS all fail: result sets that cannot be streamed have no route

1 participant