TCP N11: query parameter binding — {name:Type} values over the native protocol

[alex-clickhouse]

Stacked on #560 (tcp/epic-m-connection-pool) — review that one first; this PR's diff is the parameters only.

TCP epic N11: bind values to a parameterized query's {name:Type} placeholders. The Query packet has carried a parameter-list field since L4, but the client always sent it empty.

await using var client = new ClickHouseTcpClient("Host=localhost");

var options = new ClickHouseTcpQueryOptions
{
    Parameters = new ClickHouseTcpParameterCollection
    {
        { "since", new DateOnly(2026, 1, 1) },
        { "tags",  new[] { "a", "b" } },
    },
};

await foreach (object[] row in client.QueryAsync(
    "SELECT * FROM events WHERE day >= {since:Date} AND tag IN {tags:Array(String)}", options))
{
}

No method signature changed — every overload already took an options object, so only four internal parameters: null call sites moved. InsertAsync inherits Parameters from the same record, so INSERT INTO t SELECT ... WHERE x > {t:Int32} works with no extra plumbing.

The wire contract is not what the spec said

This is the part worth reading. Our native-protocol spec note (ignored-docs/tcp/native-protocol.md, not in the repo) claimed that String parameters must arrive single-quoted. The truth is broader and sharper, and I established it by probing ~60 cases against ClickHouse 26.6.1 on both transports before writing any code.

Every type must be quoted. The parameter rides the settings list with flags = 0x02 (Custom), so the server runs Field::restoreFromDump on the text before the {name:Type} substitution ever sees it. An unquoted 42 for an Int32 parameter fails with Couldn't restore Field from dump: 42, exactly as a bare Alice does for a String.

The value therefore crosses two unescape stages, and stage 2 turns out to be the same escaped-text reader that HTTP's param_<name> feeds — I confirmed that by curling the HTTP endpoint and getting byte-identical results. So the rule is:

wire value = QuoteSingle(Escape(T))     where T is the exact text HTTP sends

To send the three characters a\b the wire carries 'a\\\\b'. A literal newline must travel as 'a\\nb', because stage 2 terminates on a raw newline or tab.

Two server behaviours here are silent rather than diagnosed, and both corrupt data:

• Doubling a quote does not escape it. 'a''b' restores as a, no error. Only backslash escaping works.
• Trailing text after the closing quote is ignored. 'ab' XYZ restores as ab.

Neither is reachable through this API — the formatter owns the quoting — but they are why the escaping is not negotiable, and why ParameterText.Escape is applied twice rather than once. The spec note and the L4 TODO entry are corrected.

Decisions worth a look

The formatter is a deliberate duplicate, and that is the uncomfortable part. Since the inner text must match HTTP byte for byte, the ideal is one implementation. The assembly graph forbids it: ClickHouse.Driver already references ClickHouse.Driver.Tcp, so a reference back is circular, and HttpParameterFormatter is written against the RowBinary ClickHouseType tree, which this assembly does not have. Sharing would mean moving ClickHouseType, TypeConverter and TypeSettings into ClickHouse.Driver.Common while keeping the net6.0 HTTP build green — an epic, not a PR. So TcpParameterFormatter mirrors HttpParameterFormatter arm for arm, and nothing enforces that today. A CI parity test (one shared case table through both formatters, asserting tcp == http.Escape().QuoteSingle()) is filed as TODO N11a rather than built here. It is also the precondition for ever collapsing the two.

Five defects found by comparing against the other two implementations. I first wrote this port to reproduce two known HTTP bugs on the grounds that parity was the invariant. That was the wrong call — parity with a bug is still a bug — so they are fixed, and I then went looking for more by diffing this formatter's behaviour against the HTTP test corpus and against clickhouse-go's native parameter tests.

They are fixed on the TCP side only. This PR touches nothing outside ClickHouse.Driver.Tcp and its tests: HTTP is a shipped transport, and changing its behaviour does not belong in a TCP epic PR. An earlier revision of this branch did fix them there too; that is reverted.

Two are inherited HTTP defects:

• A byte[] bound to a String parameter was sent as the literal text System.Byte[]. Only FixedString read the bytes. Silent corruption, and reachable in practice since readStringsAsByteArrays makes that a plausible round-trip.
• Interval<Unit> threw. TCP now sends the underlying Int64 count — the server accepts it, so this was purely a missing arm. HTTP could not be fixed the same way regardless: the RowBinary tree has no Interval type at all, so an HTTP query fails while resolving the name, before any formatter runs. Adding it is a feature with a read path and a PublicAPI change (there is already a TODO: following interval implementation at Types/BinaryTypeDecoder.cs:136).

Three more were this port being behind HTTP, which the comparison is what surfaced:

• {p:Json} threw — the arm matched only the uppercase JSON, though the server takes either spelling and reports both as JSON.
• QBit(T, N) had no arm.
• The six geo names had no arm (Point, Ring, LineString, Polygon, MultiLineString, MultiPolygon). They now format as the shape they stand for, mirroring HTTP, where PointType : TupleType and the rest are ArrayType.

All eight of those types now have live round-trips.

Type resolution has three rungs: the parameter's own ClickHouseType, then the query's {name:Type}, then the CLR value. The middle rung is a port of the HTTP SqlParameterTypeExtractor, so comments and string literals are skipped. The third rung looks redundant — a query the server can run must declare the type anyway — and it nearly is. It exists so that a parameter the query does not name, or one whose hint the scanner did not see, is harmless rather than fatal. Throwing there would have broken reusing one collection across queries.

Deliberately out of scope: ADO-style @name rewriting (this client is not an ADO provider; revisit if a TCP-backed DbConnection lands) and the IParameterFormatter / IParameterTypeResolver hooks (both live in the HTTP assembly and cannot be referenced from here).

No changelog entry, and no diff outside the TCP projects. No commit in this epic series has touched CHANGELOG.md, since the TCP client is unreleased and [Experimental], and that holds here. Say the word if you want the whole epic summarised there when it lands.

The two formatters now diverge in eight places, all recorded in N11a. Six are HTTP defects that this port fixes only on its own side; two of those silently corrupt data (byte[] as String, and a byte[] that is not valid UTF-8). The TODO carries a table of what HTTP does today and what fixing each would take, so the parity check has something concrete to settle rather than a switch to flip. Fixing them is a separate PR against main, for whoever wants to own that behavioural change.

A silent timezone defect, now refused rather than sent. A bare {d:DateTime} hint declares no timezone, so the server reads the wall-clock text in session_timezone. The client wrote a kinded DateTime/DateTimeOffset as UTC, so on a non-UTC session the instant moved — nine hours for Asia/Tokyo — with no error. The client now throws instead, naming the value's kind, what the server would have done, and both ways out:

Parameter 'd' (type DateTime): A DateTimeOffset names an instant, but the type declares no timezone, so the instant cannot be sent without loss. The server reads the value in its session timezone, which moves the instant when that is not UTC, and reports no error. Declare the timezone in the type — DateTime('UTC') — or pass a DateTime with Kind=Unspecified to send a wall-clock time for the server to read in its own timezone.

Kind=Unspecified with a bare hint stays legal, and that limit is the point. Unspecified is a wall clock with no instant attached, which is exactly what a type with no timezone carries — nothing is lost and the round-trip is faithful. Only a value that names an instant is refused, because only then does the client hold information the wire cannot carry.

Epoch was the first candidate and the evidence killed it. clickhouse-go sends epoch seconds, which is immune to session_timezone — I confirmed that. But it cannot carry the range: the server reads an integer of four digits or fewer as a year, so epoch 0–9999 (1970-01-01 00:00:00 to 02:46:39 UTC) is rejected with value 1000 cannot be parsed as Date, as is any negative epoch for DateTime. An encoding with a hole in it is not an encoding. The other candidate — formatting into the session timezone — needs the client to guess server state at format time and go silently wrong when the guess is stale. Refusing beat guessing.

HTTP keeps the old behaviour, deliberately. The distinction is not timidity: on HTTP this combination is correct today for everyone on a UTC server, which is the default. Throwing there would break working code for the majority to protect the non-UTC minority, and it is a shipped client. TCP is unreleased and [Experimental], so establishing "declare your timezone" costs nobody. Filed as N11d-http with three graded options (log a warning / opt-in switch / throw), and a note that the kinded half has no HTTP test at all — that should be written first, because it fails silently today.

Security

The Identifier arm is the only one that emits its value unescaped, so I checked what it can reach. The server substitutes it as a single AST identifier and applies its own backtick quoting, so it cannot break out:

value for {c:Identifier}	result
a) UNION ALL (SELECT 999	UNKNOWN_IDENTIFIER, treated as one name
a`,`b	escaped to a\`,\`b, one name
system.tables	one name — it cannot even cross a database

Ordinary values are inert too: ' OR 1=1 -- round-trips as data, covered by a test at both the formatter and the live-server level. An Identifier parameter is still a "trusted name" surface in the same sense it is on HTTP — it selects a name, so a caller must not let an untrusted value choose which column is read — but it cannot inject syntax.

Testing

2462 tests pass, 101 of them live-server round-trips covering every supported parameter type. Coverage on the new files: formatter 98%, extractor 98%, collection 100%, inference 87%.

The escaping expectations are hand-derived from the two-stage rule and then confirmed against a real server, so the unit and integration layers check each other rather than restating one belief twice. The character-class cases are the ones I would keep: a carriage return and a NUL need no escape while a newline and a tab do, which is easy to get wrong in either direction.

The corner cases came from reading the other two implementations' suites rather than from imagination. I inventoried what ClickHouse.Driver.Tests binds as an HTTP parameter and what clickhouse-go binds as a native one, then took everything either had that this did not. The additions a live server can settle:

• NaN, +Infinity, -Infinity — .NET and ClickHouse spell these differently and the server happens to accept both, which only a round-trip shows;
• a Bool inside a composite — the server takes 1/0 for a scalar Bool but rejects them for Array(Bool), so a formatter emitting digits passes the scalar case and fails this one (clickhouse-go hit exactly this, issue #1891);
• the Date and Date32 range ends, a pre-epoch DateTime64, and scale 9;
• an Enum by number rather than by label; decimals of 30 and 50 digits, past what a CLR decimal holds;
• negative and past-24h Time/Time64; a surrogate pair; a nine-element ValueTuple (TRest nesting);
• four more Map shapes — empty, a key needing escapes, a non-string key, one holding an array.

Non-zero-lower-bound and zero-length array axes are unit tests, being shapes no server round-trip can produce.

The suite can now gate on server version, which QBit forced: it reached the matrix and failed on 25.8, which predates the type. TcpFeature carries a [SinceVersion] per capability and TcpServerFeatures maps a version onto the set — the same shape as Feature and ClickHouseFeatureMap in the HTTP suite, kept as a separate copy because the TCP projects do not reference ClickHouse.Driver, which is what keeps the TCP CI job building only the two TCP projects. Whole tests use [RequiresServerFeature]; a single case of a TestCaseSource guards its yield, since a case that is never yielded cannot carry an attribute. The version comes from CLICKHOUSE_VERSION rather than from the server, because a case source is enumerated during discovery, before the fixture has started the container. A moving tag resolves to "everything supported", so a real gap fails loudly instead of being skipped in silence. The mapping has its own tests — a wrong answer there does not fail a test, it skips one.

The comparison also found gaps in the HTTP suite that I did not fix here, filed as N11c: no NaN/Infinity parameter anywhere, no upper-bound dates (only the lower bound is ever tested), no DST-gap or DST-overlap instant, and Time/Time64 commented out of the composite matrix. Each is two tests, and the HTTP half may well find something.

Second review round

Six inline comments, all valid against current HEAD — none of them outdated. Four defects, plus two of my own tests that passed against the code they were meant to guard:

• The SQL scanner ignored \'. ClickHouse accepts a backslash-escaped quote in a string literal, so a literal holding one looked closed and the placeholder after it was skipped — the type then fell back silently to CLR inference.
• Variant alternatives were matched on the outer type name alone, so a string[] took the first arm of Variant(Array(Int32), Array(String)) and every element went out unquoted. Matching now recurses into element, key/value and tuple element types, and checks tuple arity.
• A Map read back from this client could not be bound again. The codec surfaces a row as KeyValuePair[] so duplicate keys and order survive, and only IDictionary was accepted.
• byte[] was decoded as UTF-8, so 0xFF became U+FFFD and the server stored EF BF BD. Probing showed \xHH in the inner text yields the exact byte, so bytes that are not valid UTF-8 now travel that way while valid UTF-8 keeps the readable form.
• The InsertAsync test called ExecuteAsync. A dropped parameters argument in either real overload would have passed the whole suite. Both are covered now, through a parameterized target.
• The one-shot sequence test was doubly vacuous: its SQL carried a hint, so inference never ran, and an iterator method yields a fresh enumerator per enumeration, so it was not one-shot either. Coverage moved to BuildParameters, where SQL that does not name the parameter reaches the inference pass.

Since the sharpest finding was "this test passes on the unfixed code", I reverted all four source fixes and re-ran. Fifteen new tests failed, as they should — and two of mine passed, so I had repeated the mistake while fixing it. One put the literal after the placeholder, so the scanner never entered a string on the way to the hint; with that corrected it still passed, because an even number of escaped quotes makes the mis-scan open and close the same number of times and land outside a string again. The third attempt showed the difference is not observable through the server at all, so that test is gone and a comment points at the unit tests, which are true negatives. Every new test here has been run against the unfixed code and fails there.

Coverage then flagged that the Variant pair-sequence arm had no test at all, and that a try/catch (FormatException) I had added around NamedElementParser.Split can never fire — the exception comes from ElementTypeStrings, which validates arity. Both fixed; ParameterTypeInference 82% → 95%.

Review outcomes

This is the earlier review pass, before the cross-implementation comparison above. Five defects found and fixed. Four were parity divergences, and one of those was silent:

• A byte[] in a Variant became the text System.Byte[]. It matched a string alternative before an array one, and the string arm prints the CLR type. HTTP picks Array(UInt8) via CanWrite. This was the only silent corruption found — everything else failed loudly.
• Time64 rounded away from zero where HTTP pre-rounds to even, so a midpoint landed one tick high.
• A decimal given as text rejected the exponent, thousands-separator and accounting-parentheses forms HTTP accepts.
• A Variant error named a parameter the caller never wrote, because alternative matching inferred under a placeholder name.
• Type inference consumed a one-shot sequence: it read the sequence to find the element type and formatting read it again, so a LINQ chain or iterator arrived empty. Copied before the first read now.

Plus the multidimensional-array depth check I found myself before the review landed: the arm peeled the declared nesting only as far as the CLR rank, so a rank-3 array declared Array(Array(T)) emitted three bracket levels and only the server noticed.

One trap the client cannot fix, and CI corrected my understanding of it. A parameter named after a server setting can be applied as that setting, because the native protocol carries parameters in the settings list. limit and offset are the names callers reach for, and the failure is a parse error about a quoted string that names neither the parameter nor the cause.

I first wrote this up as a fixed protocol limitation, on the evidence of my local 26.6.1. The matrix disagreed: it is version-dependent and already fixed upstream — 25.8 through 26.6 reject the query, latest binds correctly. My local server happened to sit on the broken side, so nothing short of the matrix would have shown it.

The test therefore pins the property that holds on every version rather than one version's error: such a name must never bind to the wrong value. Either the server rejects the query, or the count is right; a wrong count fails. That is the outcome that would actually hurt, and it is a stronger assertion than the one it replaced. The doc now says "rename the parameter if you support any server in 25.8–26.6" instead of claiming the protocol forbids it. Unaffected on HTTP, which carries the name separately.

The review also confirmed the parts I most wanted checked: the two-stage escaping held against every adversarial input it tried (BEL/BS/FF/VT, an already-quoted value, a 200 000-character value, literal \x41 and \N sequences, emoji), the Identifier arm is contained by the outer escape, and the extractor is a faithful port.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds native TCP binding for ClickHouse {name:Type} query parameters.

Changes:

• Adds parameter APIs and query/insert plumbing.
• Implements type extraction, inference, formatting, and wire escaping.
• Adds protocol, unit, and integration coverage.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
TcpParameterFormatter.cs	Formats parameter values for native transport.
SqlParameterTypeExtractor.cs	Extracts type hints from SQL.
ParameterTypeInference.cs	Infers types from CLR values.
ParameterText.cs	Implements two-stage escaping.
IClickHouseTcpClient.cs	Documents parameter support.
ClickHouseTcpQueryOptions.cs	Exposes per-query parameters.
ClickHouseTcpParameterCollection.cs	Adds ordered parameter collection.
ClickHouseTcpParameter.cs	Defines a bound parameter.
ClickHouseTcpClient.cs	Forwards formatted parameters.
QueryPacketTests.cs	Verifies wire encoding.
TcpParameterFormatterTests.cs	Tests standard formatting.
TcpParameterFormatterEdgeCaseTests.cs	Tests formatter edge cases.
SqlParameterTypeExtractorTests.cs	Tests SQL hint extraction.
ClickHouseTcpParameterCollectionTests.cs	Tests collection and resolution.
ClickHouseTcpParameterIntegrationTests.cs	Adds live-server coverage.

Suppressed comments (1)

ClickHouse.Driver.Tcp/Parameters/SqlParameterTypeExtractor.cs:147

• The same scanner bug occurs inside quoted type arguments: valid backslash-escaped enum labels are terminated at \', so a } in the label can truncate the extracted type and make TypeParser.Parse reject a query the server accepts. Handle backslash escapes here too; TypeTokenizer already treats them opaquely.

                // An escaped quote ('') stays inside the string.
                if (c == '\'' && i + 1 < sql.Length && sql[i + 1] == '\'')
                {
                    i += 2;
                    continue;

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.

Suppressed comments (6)

ClickHouse.Driver.Tcp/Parameters/ParameterTypeInference.cs:102

• This does not actually make an array alternative take precedence for byte[]: Variant(String, Array(UInt8)) accepts the first String arm and decodes the bytes as UTF-8, whereas the HTTP CanWrite path selects Array(UInt8) regardless of ordering. The existing test only uses Variant(Array(UInt8), String), so it misses the corrupting order. Prefer an array-compatible alternative for byte[] before falling back to String/FixedString.

            // A byte array reads as text or as an array of bytes, so an Array alternative takes it first. The
            // string arms would otherwise win and print the CLR type name instead of the contents.
            byte[] => node.Name is "Array" or "String" or "FixedString" ? node.Name : "String",

ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpParameterIntegrationTests.cs:283

• This test supplies {p:Array(Int32)}, so BuildParameters finds a type hint and never calls Materialize or ParameterTypeInference.Infer; the iterator is only enumerated once and the test passes without the one-shot-sequence fix. Add a unit regression that calls BuildParameters with an unreferenced parameter (for example SQL SELECT 1) and asserts the inferred/formatted wire value.

        object value = await ScalarAsync(client, "SELECT toString({p:Array(Int32)})", options);

ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpParameterIntegrationTests.cs:421

• Despite the test name, this invokes ExecuteAsync, so it only covers the query path and none of the three newly wired InsertAsync call sites. Exercise an actual InsertAsync overload with parameters (for example a {target:Identifier} table placeholder) so a regression that drops parameters from inserts fails.

            await client.ExecuteAsync($"INSERT INTO {table} SELECT number FROM numbers(5) WHERE number > {{threshold:Int32}}", options, None);

ClickHouse.Driver.Tcp/Parameters/SqlParameterTypeExtractor.cs:42

• ClickHouse also supports backslash-escaped quotes in SQL strings. For a valid literal such as 'it\'s {p:Date}', this scanner treats the escaped quote as the end of the string and extracts the placeholder-looking text inside it; a later real {p:Int32} can then be skipped or reported as conflicting. Skip a backslash and its escaped character in both quote-scanning loops, and add a regression case.

                // An escaped quote ('') stays inside the string.
                if (c == '\'' && i + 1 < sql.Length && sql[i + 1] == '\'')

ClickHouse.Driver.Tcp/Parameters/TcpParameterFormatter.cs:506

• Round the total duration before splitting it into hours/minutes/seconds. At a carry boundary such as 00:00:59.9995 for Time64(3), this currently emits 0:00:60.000, whose seconds component is outside ClickHouse's 00..59 range, instead of 0:01:00.000.

        // Round before formatting, and to even, because decimal.ToString rounds away from zero. Without this a
        // midpoint lands one tick above where the HTTP formatter puts it.
        string secondsText = Math.Round(seconds, scale, MidpointRounding.ToEven)
            .ToString("00." + new string('0', scale), CultureInfo.InvariantCulture);

ClickHouse.Driver.Tcp/Parameters/ParameterTypeInference.cs:106

• TimeSpan is omitted from the shared-CLR-type cases even though the formatter accepts it for both Time and Time64. Consequently a valid value for Variant(Time, String) is rejected as having no matching alternative because fallback inference only returns Time64. Match TimeSpan against either time type, as the HTTP CanWrite path does.

            DateTime or DateTimeOffset => node.Name is "DateTime" or "DateTime64" or "Date" or "Date32" ? node.Name : "DateTime64",