Repository navigation
Fix ClickHouseConnection: PostStreamAsync and ClickHouseBulkCopy ignore ChangeDatabase - #684
Open
polyglotAI-bot wants to merge 2 commits into
Open
polyglotAI-bot wants to merge 2 commits into
polyglotAI-bot wants to merge 2 commits into
Conversation
…re ChangeDatabase ChangeDatabase stores the database on the connection only. ClickHouseClient.CreateUriBuilder falls back to Settings.Database when the request options carry no database. ClickHouseCommand passes connection.Database, but PostStreamAsync passed only the query id (or no options at all), and ClickHouseBulkCopy built its InsertOptions without a database. Both paths therefore ran in the configured database, so an unqualified table name could resolve to a table in the wrong database. PostStreamAsync now always sends the connection's Database, and ClickHouseBulkCopy reads connection.Database at write time for the schema probe and the INSERT. Fixes: #683 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
polyglotAI-bot
requested review from
alex-clickhouse and
mzitnik
as code owners
October 15, 2026 05:20
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The public behavior correction still needs the required update to the HTTP user guide.
1 open finding
What changed in this PR
Fixes #683 in the HTTP client so stream requests and bulk copies respect the database selected by ChangeDatabase.
Changes:
- Pass the connection’s selected database through query and insert options.
- Add regression tests for both stream overloads and binary insert formats.
- Add a changelog fragment.
| File | Description |
|---|---|
| ClickHouse.Driver/Copy/ClickHouseBulkCopy.cs | Selects the database at write time. |
| ClickHouse.Driver/ADO/ClickHouseConnection.cs | Includes the selected database in stream request options. |
| ClickHouse.Driver.Tests/BulkCopy/BulkCopyTests.cs | Tests database selection for both binary formats. |
| ClickHouse.Driver.Tests/ADO/ConnectionTests.cs | Tests both stream overloads with and without query IDs. |
| changelog.d/683-change-database-post-stream-bulk-copy.fixes.md | Records the database-routing correction. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Document that commands, both connection PostStreamAsync overloads, and ClickHouseBulkCopy use the database selected with ChangeDatabase, and that the bulk copy reads it when WriteToServerAsync runs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
5 tasks done
This branch has not been deployed
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 #683.
ChangeDatabasestores the database on the connection only.ClickHouseClient.CreateUriBuilderusesqueryOverride?.Database ?? Settings.Database.ClickHouseCommandpassesconnection.Database, but two other paths that go through the connection did not:ClickHouseConnection.PostStreamAsync(both overloads) passed options with onlyQueryId, ornullwhen no query id was given.ClickHouseBulkCopy.WriteToServerAsyncbuilt itsInsertOptionswithoutDatabase.Both paths therefore ran in the configured database. An unqualified table name resolved in the wrong database: the call failed with
UNKNOWN_TABLE, or, if a table with the same name existed there, the rows went into it with no error.Changes
ClickHouseConnection.PostStreamAsync: always buildQueryOptionswithDatabase = Database(plusQueryIdwhen given). All other options staynull, so the request is the same as before apart from the database.ClickHouseBulkCopy.WriteToServerAsync: setDatabase = connection.Databaseon theInsertOptions. It is read at write time, so aChangeDatabaseafter the bulk copy is constructed also applies.InsertOptions.Databasedrives both the schema probe and theINSERT.changelog.d/683-change-database-post-stream-bulk-copy.fixes.md.Without
ChangeDatabase,DatabasereturnsSettings.Database, so those requests do not change.ClickHouseBulkCopy(string connectionString)owns its connection, so it is not affected.Tests
ConnectionTests.PostStreamAsync_AfterChangeDatabase_RunsInSelectedDatabase: both overloads, with and without a query id (the two branches of the old options helper). RunsSELECT currentDatabase()afterChangeDatabase("system"). Onmainall 4 cases returndefault.BulkCopyTests.WriteToServerAsync_AfterChangeDatabase_InsertsIntoSelectedDatabase:RowBinaryandRowBinaryWithDefaults. The table exists only intest_secondaryand the bulk copy gets the bare name. Onmainboth cases fail withUNKNOWN_TABLEfrom the schema probe.The new tests fail on
mainand pass with the fix. FullClickHouse.Driver.Testsrun on net10.0 against ClickHouse 26.9: 14825 passed, 0 failed, no existing tests changed.Related, not changed here:
Clone()also drops the database chosen withChangeDatabase(#682).Checklist
🤖 Generated with Claude Code