Skip to content

fix(jdbc-v2): throw on closed connection in commit, rollback, getCatalog, getNetworkTimeout, setClientInfo - #3281

Merged
chernser merged 3 commits into
ClickHouse:mainfrom
pcbeingused333:fix/jdbc-v2-closed-connection-checks
Nov 3, 2026
Merged

chernser merged 3 commits into
ClickHouse:mainfrom
pcbeingused333:fix/jdbc-v2-closed-connection-checks

Conversation

@pcbeingused333

@pcbeingused333 pcbeingused333 commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #3270

After Connection#close(), several jdbc-v2 ConnectionImpl methods did not check the connection state: getCatalog() and getNetworkTimeout() returned values, setClientInfo(...) updated state, and commit()/rollback() only reported the unsupported feature (and did nothing with jdbc_ignore_unsupported_values=true). The JDBC contract requires an SQLException on a closed connection.

ensureOpen() is now the first statement of commit(), rollback(), getCatalog() and getNetworkTimeout(). Both setClientInfo overloads only declare SQLClientInfoException, so they use a small helper that throws it with the same message and SQLState (08000) as ensureOpen(). close(), abort(), isClosed() and isValid() are unchanged.

Compatibility: Calls on an already closed connection now throw instead of returning silently. Code that relied on those calls succeeding after close() would now see an exception; open connections are unaffected. A history/latest/3270.md entry is included.

Note: #3266 touches the body of getNetworkTimeout(); the overlap is a one-line addition at the top of the method.

Test: ConnectionTest#testClosedConnectionThrows closes a connection (with defaults and with jdbc_ignore_unsupported_values=true) and asserts each method throws, while close(), isClosed() and isValid() stay allowed. It fails on main (nothing thrown) and passes with the fix, against a ClickHouse server.

Checklist

This change was prepared with the assistance of an AI coding tool (Claude Code) and reviewed before submission.

🤖 Generated with Claude Code


Note

Medium Risk
Behavior change on closed connections may break code that relied on silent no-ops, but open-connection behavior is unchanged and aligns with the JDBC contract.

Overview
Fixes #3270 by enforcing JDBC behavior after Connection#close() in jdbc-v2 ConnectionImpl.

commit(), rollback(), getCatalog(), and getNetworkTimeout() now call ensureOpen() first, so a closed connection raises SQLException with SQLState 08000 ("Connection is closed") instead of returning values or only hitting the unsupported-feature path (including when jdbc_ignore_unsupported_values=true).

Both setClientInfo overloads use a new ensureOpenForClientInfo() helper that throws SQLClientInfoException with the same message and SQLState. isClosed() no longer declares throws SQLException, matching the Connection interface. close(), isValid(), and related lifecycle behavior are unchanged.

Adds ConnectionTest#testClosedConnectionThrows (with and without ignore-unsupported) and a history/latest/3270.md changelog note. Compatibility: callers that invoked these methods on an already-closed connection will now get exceptions instead of silent success.

Reviewed by Cursor Bugbot for commit 8040bc3. Bugbot is set up for automated code reviews on this repo. Configure here.

…log, getNetworkTimeout, setClientInfo

Fixes ClickHouse#3270

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Alex Castillo <alex.castillog33@gmail.com>
@CLAassistant

CLAassistant commented Oct 31, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@chernser

Copy link
Copy Markdown
Contributor

@pcbeingused333
thank you for the contribution!
please sign CLA.

@pcbeingused333

Copy link
Copy Markdown
Contributor Author

Done, ensureOpenForClientInfo now uses isClosed() (wrapping its SQLException into SQLClientInfoException).

This comment was drafted with Claude Code.

@chernser chernser left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments.

@chernser
chernser merged commit 70c976c into ClickHouse:main Nov 3, 2026
24 of 28 checks passed
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.

[jdbc-v2] Connection#getNetworkTimeout, getCatalog and setClientInfo do not throw on a closed connection

3 participants