[jdbc-v2] Fix Statement.cancel() for query run with session_id parameters #2868
Merged
Conversation
|
Repository collaborators can run the JMH benchmark suite against this PR by commenting: Optional regression threshold override (Δ% on Time or Alloc/op; defaults to 10%): Only one benchmark run per PR is active at a time — issuing a new |
Client V2 CoverageCoverage Report
Class Coverage
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
|
Contributor
Author
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit df1a530. Configure here.
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
2 participants
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
Initial problem is that JDBC driver uses
session_idfor all queries even control ones like incancel()method. If session is set thencancel()causes server errorSESSION_IS_LOCKEDerror. Correct way is to remove session settings fromKILLquery.Main changes in this PR are:
clearSession()in QuerySettings and other operations. This method holds all session reset logic and removes session related settings like id, check, timeout. Timezone session is not removed because it can be used without session ID and used to set particular timezone for request.StatementImpl.cancel()that clones local settings and clear session. Then settings are used forKILLquery.Closes #2690
Checklist
Delete items not relevant to your PR:
Note
Low Risk
Targeted cancel-path change with new session-clear API and regression tests; no auth or data-path changes beyond control queries.
Overview
Fixes jdbc-v2
Statement.cancel()when the statement uses a ClickHouse session (session_id/clickhouse_setting_session_id): the internalKILL QUERYno longer inherits session settings, avoiding server error SESSION_IS_LOCKED (“Session is locked by a concurrent client”).client-v2 adds
clearSession()onCommonSettingsand fluent wrappers onQuerySettings,InsertSettings, andCommandSettings. It clearssession_id,session_check, andsession_timeoutonly;session_timezoneis left intact so timezone can still apply without a session.StatementImpl.cancel()merges statement-local settings into a copy and callsclearSession()before issuingKILL QUERY(includingON CLUSTERwhen configured).Unit tests cover
clearSession()behavior; integration tests cancel long-running SELECT and INSERT inside a session. CHANGELOG documents the fix under 0.10.0.Reviewed by Cursor Bugbot for commit df1a530. Bugbot is set up for automated code reviews on this repo. Configure here.