Skip to content

Fix the client losing its default database after USE {db:Identifier} - #111982

Merged
alexey-milovidov merged 3 commits into
masterfrom
fix-use-query-parameter-default-database
Jul 28, 2026
Merged

Fix the client losing its default database after USE {db:Identifier}#111982
alexey-milovidov merged 3 commits into
masterfrom
fix-use-query-parameter-default-database

Conversation

@alexey-milovidov

@alexey-milovidov alexey-milovidov commented Jul 26, 2026

Copy link
Copy Markdown
Member

USE {db:Identifier} keeps the database name in a query parameter, so the parsed ASTUseQuery carries an empty name and ASTUseQuery::getDatabase returns an empty string until the parameters are substituted. The client used that empty name to update its default_database, which it needs to restore the current database whenever it has to re-establish a lost connection. So after a USE with a query parameter, a reconnect silently moved the session to the default database, and every later unqualified table reference failed with UNKNOWN_TABLE.

The fix substitutes the query parameters on the client side, the same way the server does when it executes the query, and the same way the client already does for SET.

This is what made stateless tests that combine USE {CLICKHOUSE_DATABASE:Identifier} with unqualified table names flaky - the client re-establishes the connection on its own when a ping does not come back in time, which happens on a loaded sanitizer runner. 03101_analyzer_identifiers_4 failed on master with:

Code: 60. DB::Exception: Unknown table expression identifier 't' in scope SELECT COLUMNS('^c') FROM t. Maybe you meant test_qp0dhise.t?. (UNKNOWN_TABLE)

and the server log shows the client dropping the connection mid-script and coming back without a database:

19:13:38.325 TCPHandler: Connected ClickHouse client version 26.8.0, revision: 54488, database: test_qp0dhise, user: default.
...
19:13:55.417 TCPHandler: Connected ClickHouse client version 26.8.0, revision: 54488, user: default.
19:13:58.349 executeQuery: Code: 60. ... Unknown table expression identifier 't' ...

The same failure mode hit 03101_analyzer_identifiers_1 (four occurrences of the two tests over the last three weeks, always UNKNOWN_TABLE on an unqualified name).

The new test 04647_use_query_parameter_survives_reconnect is an expect test: it lets an interactive client sit idle for several seconds with idle_connection_timeout = 2, so the server closes the connection long before the client checks it and the reconnection happens deterministically. It fails with UNKNOWN_TABLE before this change and passes after it.

Note that idle_connection_timeout has to be passed on the command line rather than with SET: the server extracts the connection settings when it receives a query, so an in-session SET idle_connection_timeout only takes effect from the next query and does not shorten the idle period that follows the SET itself. The first version of the test used SET idle_connection_timeout = 0, which closes the connection immediately after every query - including right after the Ping that the client sends before a query - so the query went into an already closed socket and the client reported NETWORK_ERROR or ATTEMPT_TO_READ_AFTER_EOF instead of reconnecting. That made the test itself flaky.

Note that 03101_analyzer_identifiers_2 fails the same way on master, but for a different reason that this change does not address: it uses CREATE TEMPORARY TABLE, and temporary tables live in the session, so they cannot survive a reconnect at all.

CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?REF=master&sha=a58de9c1a55e6577c3a6cafdac5609bc90234ef0&name_0=MasterCI&name_1=Stateless%20tests%20%28amd_msan%2C%20WasmEdge%2C%20parallel%2C%202%2F2%29

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fixed clickhouse-client losing the current database after USE with a query parameter, e.g. USE {db:Identifier}. If the client had to re-establish the connection later, it silently switched the session to the default database, and subsequent queries using unqualified table names failed with UNKNOWN_TABLE.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

Version info

  • Merged into: 26.8.1.294 (included in 26.8 and later)

`USE {db:Identifier}` keeps the database name in a query parameter, so the
parsed `ASTUseQuery` carries an empty name and `ASTUseQuery::getDatabase`
returns an empty string until the parameters are substituted. The client used
that empty name to update its `default_database`, which it needs to restore the
current database whenever it has to re-establish a lost connection. So after a
`USE` with a query parameter, a reconnect silently moved the session to the
`default` database, and every later unqualified table reference failed with
`UNKNOWN_TABLE`.

Substitute the query parameters on the client side, the same way the server does
when it executes the query, and the same way the client already does for `SET`.

This is what made stateless tests that combine `USE {CLICKHOUSE_DATABASE:Identifier}`
with unqualified table names flaky - the client re-establishes the connection on
its own when a ping does not come back in time, which happens on a loaded
sanitizer runner. For example `03101_analyzer_identifiers_4`:

    Code: 60. DB::Exception: Unknown table expression identifier 't' in scope
    SELECT COLUMNS('^c') FROM t. Maybe you meant test_qp0dhise.t?. (UNKNOWN_TABLE)

with the server log showing the re-established connection arriving without a
database: `Connected ClickHouse client version 26.8.0, revision: 54488, user: default.`

https://s3.amazonaws.com/clickhouse-test-reports/json.html?REF=master&sha=a58de9c1a55e6577c3a6cafdac5609bc90234ef0&name_0=MasterCI&name_1=Stateless%20tests%20%28amd_msan%2C%20WasmEdge%2C%20parallel%2C%202%2F2%29
@clickhouse-gh

clickhouse-gh Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Workflow [PR], commit [ab4ab4e]

Summary:


AI Review

Summary

This PR fixes clickhouse-client so a successful USE {db:Identifier} updates the client's remembered default database using the substituted identifier, which prevents reconnects from silently falling back to default. I reviewed the current diff, the surrounding reconnect and query-parameter code paths, the new expect regression test, the existing PR discussion, and the current CI state, and I did not find a correctness, safety, or coverage issue that warrants changes.

Final Verdict
  • Status: ✅ Approve

@clickhouse-gh clickhouse-gh Bot added the pr-bugfix Pull request with bugfix, not backported by default label Jul 26, 2026
The `.sql` version of the test was flaky (it was the only failure in
eight `Stateless tests` jobs, e.g.
https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=111982&sha=17d09d1e6ac7b9ed21ec7d2857713c26c2497c33&name_0=PR&name_1=Stateless%20tests%20%28amd_msan%2C%20WasmEdge%2C%20parallel%2C%201%2F2%29
). With `idle_connection_timeout = 0` the server closes the connection
immediately after every query, including right after the `Ping` that the
client sends before a query - the query then goes into an already closed
socket and the client reports `NETWORK_ERROR` or
`ATTEMPT_TO_READ_AFTER_EOF` instead of reconnecting.

Replace it with an `expect` test that lets the client sit idle for
several seconds with `idle_connection_timeout = 2`, so the connection is
closed long before the client checks it and the reconnection is
deterministic. Verified locally: 5/5 runs pass with the fix, and the test
fails with `UNKNOWN_TABLE` without it.

Note that `idle_connection_timeout` has to be passed on the command line:
the server extracts the connection settings when it receives a query, so
an in-session `SET idle_connection_timeout` only takes effect from the
next query and does not shorten the idle period that follows the `SET`
itself.
@clickhouse-gh

clickhouse-gh Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

LLVM Coverage Report

Metric Baseline Current Δ
Lines 86.30% 86.30% +0.00%
Functions 92.00% 92.00% +0.00%
Branches 78.40% 78.40% +0.00%

Changed lines: Changed C/C++ lines covered: 6/6 (100.00%) · Uncovered code

Full report · Diff report

@alexey-milovidov alexey-milovidov left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, this is good.

@alexey-milovidov alexey-milovidov self-assigned this Jul 28, 2026
@alexey-milovidov
alexey-milovidov added this pull request to the merge queue Jul 28, 2026
Merged via the queue into master with commit 5b8d503 Jul 28, 2026
179 checks passed
@alexey-milovidov
alexey-milovidov deleted the fix-use-query-parameter-default-database branch July 28, 2026 18:06
@robot-clickhouse robot-clickhouse added the pr-synced-to-cloud The PR is synced to the cloud repo label Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix Pull request with bugfix, not backported by default pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants