TCP N: Add ClickHouseTcpClient: public client API (read path + inserts) - #462
TCP N: Add ClickHouseTcpClient: public client API (read path + inserts)#462alex-clickhouse wants to merge 4 commits into
Conversation
TriageCategory: Summary What this impacts
Concerns
Required reviewer action
|
There was a problem hiding this comment.
Pull request overview
Adds an experimental, high-level native-TCP client API (ClickHouseTcpClient) on top of the existing TCP protocol layer, including connection acquisition plumbing and client/query options, with tests covering core behaviors and live-server integration.
Changes:
- Introduces
ClickHouseTcpClientwith streaming (Block), untyped row streaming (object[]), execute/insert, ping, and per-query settings merge (incl. flattened Dynamic/JSON serialization injection). - Adds options and parsing/building support (
ClickHouseTcpClientOptions,ClickHouseTcpQueryOptions,ClickHouseTcpConnectionStringBuilder) plus a connection-source seam with an initialSingleConnectionSource. - Makes
Blockpublic and addsBlock.ColumnNames; threadsMaxSendBufferBytesthrough the TCP insert write path; adds unit + integration tests.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ClickHouse.Driver.Tcp/Protocol/ClickHouseTcpConnection.cs | Threads a send-buffer flush threshold through insert streaming and validates it. |
| ClickHouse.Driver.Tcp/Format/Block.cs | Makes Block public and adds cached ColumnNames. |
| ClickHouse.Driver.Tcp/Client/SingleConnectionSource.cs | Implements a serialized, single-connection rent/lease source with redial on terminated connections. |
| ClickHouse.Driver.Tcp/Client/IConnectionSource.cs | Defines internal connection source + lease interfaces for pooling seam. |
| ClickHouse.Driver.Tcp/Client/ClickHouseTcpQueryOptions.cs | Adds minimal per-query overrides (QueryId + settings). |
| ClickHouse.Driver.Tcp/Client/ClickHouseTcpConnectionStringBuilder.cs | Adds TCP-native connection string builder/parser (incl. set_* settings). |
| ClickHouse.Driver.Tcp/Client/ClickHouseTcpClientOptions.cs | Defines validated client-level endpoint/timeout/buffer/settings options. |
| ClickHouse.Driver.Tcp/Client/ClickHouseTcpClient.cs | Adds the experimental public TCP client API and settings merge behavior. |
| ClickHouse.Driver.Tcp.Tests/Integration/TcpServerFixture.cs | Adds fixture helpers for creating options/client and a TCP connection string. |
| ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpClientIntegrationTests.cs | Live-server integration coverage for streaming, inserts, reuse/redial, settings, and concurrency. |
| ClickHouse.Driver.Tcp.Tests/Client/SingleConnectionSourceTests.cs | Unit tests for SingleConnectionSource lifecycle and cancellation behavior. |
| ClickHouse.Driver.Tcp.Tests/Client/ClickHouseTcpConnectionStringBuilderTests.cs | Unit tests for builder parsing/defaults/custom settings/round-trip. |
| ClickHouse.Driver.Tcp.Tests/Client/ClickHouseTcpClientSettingsTests.cs | Unit tests for settings merge + flattened serialization injection behavior. |
| ClickHouse.Driver.Tcp.Tests/Client/ClickHouseTcpClientOptionsTests.cs | Unit tests for defaults, validation, and handshake mapping. |
| ClickHouse.Driver.Tcp.Tests/ClickHouse.Driver.Tcp.Tests.csproj | Suppresses the experimental API diagnostic for tests. |
ec2c0ca to
1fa7db1
Compare
|
Thanks — addressed all three (pushed as an amend to
Full net9.0 suite green (1102 tests). |
1fa7db1 to
293f045
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
ClickHouse.Driver.Tcp/Client/ClickHouseTcpClient.cs:234
- Settings dictionaries can currently contain an empty key or a null value. In the native protocol, settings are encoded as (key, flags, value) triples terminated by an empty key, so an empty key can corrupt the packet; a null value will also throw when the Query packet is written. Consider validating during merge so failures are deterministic and actionable (covers both client-level and per-query settings).
foreach (KeyValuePair<string, string> entry in clientSettings)
{
merged[entry.Key] = entry.Value;
}
293f045 to
e27b97c
Compare
| private string GetStringOrDefault(string name, string @default) | ||
| => TryGetValue(name, out object value) && value is string s ? s : @default; |
| @@ -15,14 +16,16 @@ namespace ClickHouse.Driver.Tcp.Format; | |||
| /// <c>Values.ToArray()</c>) while iterating. | |||
| var merged = new Dictionary<string, string>(StringComparer.Ordinal); | ||
| if (clientSettings is not null) | ||
| { | ||
| foreach (KeyValuePair<string, string> entry in clientSettings) | ||
| { | ||
| merged[entry.Key] = entry.Value; | ||
| } | ||
| } | ||
|
|
||
| if (perQuerySettings is not null) | ||
| { | ||
| foreach (KeyValuePair<string, string> entry in perQuerySettings) | ||
| { | ||
| merged[entry.Key] = entry.Value; | ||
| } | ||
| } |
e27b97c to
5e94cd5
Compare
5e94cd5 to
87403eb
Compare
f623015 to
9585d2a
Compare
9585d2a to
f4b752e
Compare
f4b752e to
069d77c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
0ec04c9 to
cff7c93
Compare
cff7c93 to
affd863
Compare
affd863 to
d768c40
Compare
d768c40 to
3e03228
Compare
3e03228 to
c7ba344
Compare
c7ba344 to
6dc491d
Compare
6dc491d to
8f25aeb
Compare
8f25aeb to
209b0b5
Compare
724dc65 to
9f8fdff
Compare
Introduces the first user-facing entry point for the native-TCP client on top
of the existing raw connection, plus the options and connection-acquisition
plumbing it needs.
- ClickHouseTcpClient ([Experimental("CHTCP0001")]): StreamAsync (block tier),
QueryAsync (object[] rows), ExecuteAsync (non-result statements), InsertAsync
(columnar), PingAsync. Safe to share; auto-enables the flattened
Dynamic/JSON serialization on every operation (a caller value still wins).
- ClickHouseTcpClientOptions + ClickHouseTcpConnectionStringBuilder
(Host/Port/Username/Password/Database/QuotaKey/DialTimeout/ReadTimeout/
MaxSendBufferBytes + set_<name> custom settings) and a minimal
ClickHouseTcpQueryOptions (QueryId + Settings).
- IConnectionSource/IConnectionLease seam with a single-connection interim
source that serializes access and redials a terminated connection; a real
pool implements the same interface later. DialTimeout bounds connect+handshake.
- MaxSendBufferBytes is threaded through InsertAsync as the between-column flush
threshold (the write memory backstop), independent of the block-split target.
- Block is now public (its constructor and Info stayed internal); added
Block.ColumnNames for header-order name lookup.
Covered by unit tests (options/connection-string/settings-merge/source
lifecycle) and live-server integration tests (streaming, early-dispose redial,
server-error reuse, columnar round-trip, per-query settings, Dynamic decode
without the caller setting the flag, tiny send-buffer flush, concurrency).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Asserts that writing a column from its ergonomic form produces bytes identical to writing the dense column read back from that same wire output, across Array/Nullable/Tuple/Map. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PR 462 feedback: - Validate client-level CustomSettings at construction: reject an empty/null setting name (it would collide with the empty key that terminates the wire settings list) and a null value. - ToOptions rejects a bare 'set_' key (empty setting name) and never emits a null value for a value-less set_ key. - Copy CustomSettings into an owned dictionary in the client ctor, so a caller mutating their dictionary cannot fault or partially apply a concurrent settings merge on the shared client. - Correct the Password doc: the native transport is unencrypted, so the password is not TLS-protected by this client. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…on, Block doc PR 462 (round 2): - ToOptions now formats a typed set_ value (e.g. builder["set_max_threads"] = 4) as an invariant string instead of silently dropping it to empty. - MergeSettings validates per-query settings (user-provided, unlike client CustomSettings): an empty name would truncate the wire settings list and a null value cannot be written, so both are rejected. - Document that a Block yielded by a query must not be disposed by the consumer — the reader owns its borrowed, pooled storage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
9f8fdff to
2b6fead
Compare
First branch of Epic N (client API) for the native-TCP client. Adds the user-facing entry point on top of the existing raw connection, plus the options and connection-acquisition plumbing. Stacked on
tcp/epic-j4-dynamic.What's here
ClickHouseTcpClient—[Experimental("CHTCP0001")],IAsyncDisposable, safe to share:StreamAsync→IAsyncEnumerable<Block>(low-level columnar tier)QueryAsync→IAsyncEnumerable<object[]>(untyped rows, boxed viaIColumn.GetValue)ExecuteAsync(non-result statements),InsertAsync(columnarIReadOnlyList<IColumn>),PingAsyncoutput_format_native_use_flattened_dynamic_and_json_serializationsoDynamic/JSONdecode without the caller knowing (caller value still wins).ClickHouseTcpClientOptions+ClickHouseTcpConnectionStringBuilder(Host/Port/Username/Password/Database/QuotaKey/DialTimeout/ReadTimeout/MaxSendBufferBytes+set_<name>custom settings), and a minimalClickHouseTcpQueryOptions(QueryId+Settings).IConnectionSource/IConnectionLeasewith an interimSingleConnectionSource(one connection, serialized, redials a terminated one).DialTimeoutbounds connect+handshake. A real pool (Epic M) implements the same interface with no client change.MaxSendBufferBytesthreaded throughInsertAsyncas the between-column flush threshold (write memory backstop), independent of the 50 MB block-split target.Blockis now public (constructor +Infostayed internal soBlockInfoisn't leaked); addedBlock.ColumnNames.Streaming release semantics
StreamAsyncrents a connection and returns it to the source exactly once on full drain, early enumerator disposal, or exception (anInterlockedguard prevents double-return; a terminated connection is discarded and redialed on the next rent).Tests
set_*custom settings, defaults, round-trip), settings-merge (N1a injection / caller-wins),SingleConnectionSourcelifecycle (idempotent dispose, rent-after-dispose, pre-cancelled token).object[]rows + owned-row retention,ExecuteAsyncround-trip, columnar insert round-trip, schema-mismatch, per-query settings, Dynamic decode without the caller setting the flag (proves N1a), tiny 4 KB send-buffer flushing 20k rows intact (provesMaxSendBufferBytes), concurrency, connection-string construction.Full net9.0 suite green (1101 tests). Coverage ~93% line / ~87% branch on the new code.
Deferred (called out)
ReadTimeoutis parsed/stored but not yet enforced (it is the idle read-loop deadline of Q3).internalClickHouseServerException/ClickHouseProtocolException(callers see the baseException) — exception hierarchy is Q1/Epic R.🤖 Generated with Claude Code