Repository navigation
[jdbc-v2] Do not fail Statement.cancel() when there is no running query - #3271
Merged
Merged
Conversation
ClickHouse 26.11 enables kill_throw_if_noop by default, so a local KILL QUERY that matches no running query fails with NOTHING_TO_KILL (785). Statement.cancel() wrapped this error in an SQLException, so it threw when the query was finished or not started yet. Ignore NOTHING_TO_KILL in cancel() for the sync and async client. Other KILL QUERY errors are thrown as before. Fixes: #3264 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Client V2 CoverageCoverage Report
Class Coverage
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
1 task
…d server The server returns NOTHING_TO_KILL only when `kill_throw_if_noop` is enabled, and that setting exists only since ClickHouse 26.10. On older servers (for example 25.8, which the coverage job uses) no integration test reaches the branch that ignores this error. The new test returns the NOTHING_TO_KILL response from a WireMock server, so the branch runs through the real HTTP error parsing on every server version, in sync and async modes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TriageCategory: Summary What this impacts
Concerns
Required reviewer action
|
chernser
reviewed
Oct 29, 2026
Add generic ClientUtils.findCause(throwable, type[, maxDepth]). It returns the nearest exception of the given type in the cause chain. By default it checks the throwable and up to 3 causes; the overload sets the number of causes to follow. StatementImpl.cancel() uses it instead of the inline instanceof/getCause() check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
chernser
approved these changes
Oct 29, 2026
|
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.
Summary
Fixes #3264.
Statement.cancel()in jdbc-v2 sendsKILL QUERY WHERE query_id = '<lastQueryId>'. ClickHouse 26.11 enableskill_throw_if_noopby default (ClickHouse/ClickHouse#125968, "Backward Incompatible Change"). With this setting, a localKILL QUERYthat matches no running query fails withCode: 785. NOTHING_TO_KILL.ON CLUSTERstays tolerant.cancel()wrapped every error in anSQLException. So on 26.11cancel()threw when the statement had no running query: the query was already finished (SELECT 1intestCancelOnCluster), or it was not started yet (testConcurrentCancel, wherelastQueryIdcan still benull). This is a server logic change, not a timing problem.There is nothing to cancel in this case, so the error must not break application code.
cancel()now catchesNOTHING_TO_KILLand returns after adebuglog. It does this for both shapes that the client returns: a directServerException(sync) and anExecutionExceptionwith aServerExceptioncause (async=true). All otherKILL QUERYerrors are thrown asSQLExceptionas before, for example an unknowncluster_name.The fix does not send
kill_throw_if_noop=0with theKILL QUERY, because the setting exists only since 26.10. Older servers reject unknown settings withUNKNOWN_SETTING.Changes
client-v2ServerException: added theNOTHING_TO_KILL = 785constant, next toEXECUTION_TIMEOUTand the other server error codes that the drivers use.client-v2ClientUtils: added genericfindCause(throwable, type)andfindCause(throwable, type, maxDepth). They return the nearest exception of the given type in the cause chain. By default they check the throwable and up to 3 causes;maxDepthsets the number of causes to follow (review request).jdbc-v2StatementImpl.cancel(): ignoreNOTHING_TO_KILL. It finds theServerExceptionwithClientUtils.findCause(e, ServerException.class).PreparedStatementImplgets the change through inheritance.WriterStatementImpl.cancel()does not sendKILL QUERY.docs/features.md: documented thatcancel()does nothing when the statement has no running query.history/latest/3264.md: changelog entry.Test
New
StatementTest#testCancelWhenQueryIsNotRunning, with a@DataProviderfor sync andasync=true. It callscancel()before any query and again after a finished query, then checks that the statement still runs queries. As a contrast case, it checks in both modes thatcancel()with an unknowncluster_namestill throws anSQLException. On servers 26.10 and later the test setskill_throw_if_noop=1, so it does not depend on the server default.New
StatementTest#testCancelIgnoresNothingToKillError, with the same sync/async=true@DataProvider. A WireMock server returns the realNOTHING_TO_KILLresponse (HTTP 500,X-ClickHouse-Exception-Code: 785) for theKILL QUERY, and the test checks thatcancel()does not throw and that theKILL QUERYwas sent once. The server returnsNOTHING_TO_KILLonly from 26.10 (kill_throw_if_noopdoes not exist on 25.8, 26.3 or 26.8), so on the PR matrix and on the coverage job (25.8) only this test runs the ignore branch.test_headand 26.10+ servers also run it against a real server throughtestCancelWhenQueryIsNotRunning.testCancelWhenQueryIsNotRunningfails in both rows on 26.11.1.888 (head) and on 26.10.3.7, withNOTHING_TO_KILL. The existingtestCancelOnClusterandtestConcurrentCancelfail on head with the same error, as reported in the issue.testCancelIgnoresNothingToKillErrorfails in both rows on any server version (checked on 26.9) withNOTHING_TO_KILL.StatementTesttests pass on 26.11.1.888 and on 26.10.3.7. With the mocked test added, all 109 pass on 26.9.1.1629. This includes the existing tests that cancel running queries andtestCancelOnCluster, which checks that the unknown-cluster error is still thrown.mvn -pl jdbc-v2 test(unit tests) and thePreparedStatementTestandJDBCErrorHandlingTestsintegration tests pass on head.ClientUtilsTestcases forfindCause: the default depth limit (match at 3 causes, no match at 4), an explicitmaxDepth(0 and 5), nearest match of a subtype, no match, a cyclic cause chain, and invalid arguments.ClientUtilsTest19/19 andStatementTest109/109 pass on 26.9.1.1629 with this change.Compatibility
kill_throw_if_noopis disabled: these servers do not returnNOTHING_TO_KILL.cancel()works the same as on older servers again.public static final intconstant onServerException, and two new static methods on the internalClientUtilsclass. Both are additive.Checklist (
docs/changes_checklist.md)NOTHING_TO_KILLis ignored, and only incancel(). Adebuglog records the query id and the server message. Other errors keep their type andSQLExceptionwrapping.debuglevel, not in a loop, and it contains only the query id and the server error message (no SQL values).cancel()behavior.ClientUtils.findCausehas a clear null contract (throwablecan benull,typemust not benull, negativemaxDepththrowsIllegalArgumentException) and behavior-focused unit tests. It is not a user-visible feature, sodocs/features.mddoes not list it.Pre-PR validation gate
kill_throw_if_noop=1)cancel()handles the new server error)history/latest/3264.md)AGENTS.md/docs/changes_checklist.md/docs/features.mdThe legacy v1 driver (
clickhouse-jdbcClickHouseStatementImpl.cancel()) sends the sameKILL QUERY. It is out of scope for this PR.🤖 Generated with Claude Code
Note
Medium Risk
Changes JDBC cancellation error handling for a specific server error code; behavior shift is intentional but affects apps that relied on SQLException from noop cancels on 26.11+.
Overview
jdbc-v2
Statement.cancel()no longer throws whenKILL QUERYfinds no running query. ClickHouse 26.11+ (with defaultkill_throw_if_noop) returnsNOTHING_TO_KILL(785) in that case; the driver now treats it as a no-op (debug log and return). OtherKILL QUERYfailures—including a badcluster_name—still surface asSQLException.client-v2 adds
ServerException.NOTHING_TO_KILLandClientUtils.findCause()socancel()can detect a wrappedServerExceptionin sync and async paths. Docs and changelog describe the behavior; integration and WireMock tests cover cancel-before/after-query and the ignored error code.Reviewed by Cursor Bugbot for commit e7700c3. Bugbot is set up for automated code reviews on this repo. Configure here.