Fix named Tuple/Nested columns whose element name requires backtick quoting - #504
Conversation
A JSON typed path may contain characters that need quoting, such as a space or a comma; the server accepts, stores and round-trips those columns, and reports the type as e.g. JSON(`a b` Int64). The driver could not parse that type name, so the whole query failed with "SerializationException: Unsupported path in JSON hint" — any table with such a column was unreadable, with no way to opt out. Two independent causes: * Tokenizer only treated ' as a quote character, so a comma or parenthesis inside a backtick-quoted identifier split the token before it reached JsonType.Parse. * JsonType.Parse split each hint on every space and required exactly two parts, and only trimmed backticks off the path instead of unescaping it. The path names in HintedTypes must match the raw path names on the wire, otherwise the hinted type is not applied. Fixes: #502
…uoting TypeConverter.ExtractTypeName stripped a named element's name by splitting the declaration on its first space, which cuts a backtick-quoted name in half: `p q` Int64 became "`p" / "q` Int64", so the element type resolved to "q` Int64" and the whole query failed with ArgumentException: Unknown type. The separator scan that the JSON typed-path fix introduced is factored out of JsonType into StringExtensions.IndexOfNameTypeSeparator and reused here, so the quoted identifier is skipped as a whole before the name/type separator is located.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7581fc8. Configure here.
…d JSON path Adds the JSON(`a\` b` Int64) shape (path name "a` b") to ParseShouldUnquotePathWhenPathIsBacktickQuoted. The server emits exactly this form for such a path, and it is the one shape where the escape branch of IndexOfPathTypeSeparator interacts with the separator search.
…typed-paths # Conflicts: # CHANGELOG.md # ClickHouse.Driver.Tests/Types/JsonTypeTests.cs # RELEASENOTES.md
|
There's some duplication between the two PRs with the IndexOfPathTypeSeparator, let's DRY please. |
…typed-paths # Conflicts: # CHANGELOG.md # RELEASENOTES.md
…' into polyglot/cs-quoted-tuple-element-names
The scan that skips a backtick-quoted identifier before locating the space separating a name from its type is not specific to JSON typed paths: named Tuple and Nested elements are spelled the same way and need the same scan. Keep a single implementation by exposing it as StringExtensions.IndexOfNameTypeSeparator instead of a private JsonType method, so the other call sites can reuse it rather than copy it. No behavior change for JSON hints. The shared helper additionally tolerates a hand-written identifier that doubles a backtick the way SQL does (``); the server always renders the backslash form.
…ed-tuple-element-names
|
Done — the scan now exists in exactly one place, and it is introduced once, in the base PR. Concretely: Both branches have been pushed and rebased onto the current base. Full |
Replace the parser-level type-string matrices with tests that create the column (or cast/parameterize) on a live server and read the value back, so every covered element name is one the server itself accepts and reports.
|
Done — the tests on this PR now go through the server instead of the type parser. What changed (tests only, no library change in this push):
Verification: with the fix reverted, 26 of the new cases fail (every name containing a space, across the read/Nested/wrapped/insert/parameter paths, plus all three parameter-hint spellings); with the fix, |
…ple-element-names # Conflicts: # CHANGELOG.md # ClickHouse.Driver.Tests/Types/JsonTypeTests.cs # RELEASENOTES.md
|
Rebased onto #503 (backtick-quoted JSON typed paths) landed on
Nothing was dropped: the JSON-side work this branch used to carry is now on Conflicts resolved: Still load-bearing after #503 — I re-checked rather than assuming: with Verification: build clean on |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Fixes ClickHouse type parsing for named Tuple/Nested element declarations whose element name is backtick-quoted and contains spaces (e.g. Tuple(`p q` Int64, r String)), by locating the name/type separator after a quoted identifier rather than splitting on the first space. This unblocks both reading and insert type resolution for such columns.
Changes:
- Update
TypeConverter.ExtractTypeNameto useStringExtensions.IndexOfNameTypeSeparator()for stripping named element prefixes safely with backtick-quoted names. - Add end-to-end and parse-level tests covering quoted element names across
Tuple,Nested, parameter type hints, composite wrappers, and malformed quoting. - Add release notes and changelog entries under “Unreleased → Bug Fixes”.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| ClickHouse.Driver/Types/TypeConverter.cs | Uses a quote-aware separator scan to strip named tuple/nested element names without mis-splitting backtick-quoted identifiers. |
| ClickHouse.Driver.Tests/Types/TupleTypeTests.cs | Adds coverage for quoted tuple/nested element names (read + insert paths, nested/composite cases, and malformed declarations). |
| CHANGELOG.md | Documents the bug fix in the Unreleased section. |
| RELEASENOTES.md | Mirrors the Unreleased bug-fix note for release notes consumers. |
main added 22 new Unreleased entries since this branch was cut. Each is now its own changelog.d/ fragment, extracted verbatim by line number rather than retyped, so the assembled Unreleased section reproduces main's exactly (as a set of lines; sorting by PR number reorders entries within their sections). New fragments, one per (PR, category): #390 improvements multidim blittable inserts #472 improvements per-scalar Span<byte> reads #484 fixes byte[]/TimeOnly HTTP parameters #485 fixes JSON strings under ReadStringsAsByteArrays #490 breaking raw results return compressed bytes #490 features AcceptEncoding response compression #490 improvements lz4 by default, HttpClient, errors, deflate #492 fixes HTTP response disposal #493 fixes Enum type declarations #494 fixes raw-stream double dispose #497 fixes GetSchema("Columns") restrictions #498 fixes JSON paths starting with setting names #503 fixes quoted JSON typed paths #504 fixes quoted Tuple/Nested element names #509 fixes {name:Type} scanner vs server lexer #511 fixes {name:Type} hints after a non-hint brace #513 fixes @name placeholders, heredocs, $ in names #390's entry was appended to the *released* v1.3.0 section on main (v1.3.0 shipped 2026-06-29), so it would have documented an unreleased change under a shipped version and never appeared in 1.4.0's notes. It moves to Unreleased as a fragment; the rest of v1.3.0 is byte-identical. RELEASENOTES.md regenerated with --sync-notes. `--check` passes, the solution builds, and the packed .nupkg's releaseNotes open on v1.3.0 with no Unreleased stub and no #390 bullet.
Description
ClickHouse reports a named
Tuple/Nestedelement name back-quoted whenever it needs quoting, so the type string on the wire can beTuple(`p q` Int64, r String)(verified on 26.5).TypeConverter.ExtractTypeNamestrips the element name by splitting the declaration on its first space, which cuts such a name in half:`p q` Int64becomes`p/q` Int64, the element type resolves toq` Int64, and the whole query fails withArgumentException: Unknown type. AnySELECTof a column with such a type is unreadable, and an insert into one fails the same way (the insert path resolves the destination column types through the same parser).The fix locates the name/type separator past the quoted identifier instead of at the first space, by reusing the single shared scan
StringExtensions.IndexOfNameTypeSeparator. That scan lives in #503 (it is the same scan the JSON typed-path fix needs, issue #502), so there is exactly one implementation and this PR only adds a call site.Unquoted and unnamed elements keep resolving exactly as before, and the failure mode for a malformed declaration stays the same
ArgumentException.Changes
ClickHouse.Driver/Types/TypeConverter.cs:ExtractTypeNameusesIndexOfNameTypeSeparator()instead ofSplit(" ", 2); the now-unusedSeparatorfield is removed.CHANGELOG.md/RELEASENOTES.md: entry under Unreleased → Bug Fixes.(The shared helper itself, and
JsonTypeusing it, are part of #503 — see that PR'sMove the path/type separator scan into a shared string helpercommit.)Test
ClickHouse.Driver.Tests/Types/TupleTypeTests.cs:TupleandNested: names containing a space,.+space, comma, parentheses,\``,',\n,\, doubled backticks; scalar,Decimal(10, 2),Map(String, Array(Int32))andNullable(String)element types; quoted names wrapped inArray/Map/an outerTuple; a single-element tuple; and a quoted name next to single-quoted element arguments (Enum8('x y' = 1, …),DateTime64(3, 'Europe/Amsterdam')`) so both quote kinds are honoured in one declaration.Tuple(String, Int32),Tuple(name String, age Int32),Nested(Id Nullable(String), Comment Nullable(String)), …) resolve to exactly the same element types as before.ArgumentException, for bothTupleandNested.Tuple(`p q` Int64, r String)andNested(`a b` Decimal(10, 2), c String), andInsertBinaryAsyncinto a table column of typeTuple(`p q` Int64, r String)with a read-back.15 of these fail without the
ExtractTypeNamechange (ArgumentException: Unknown type:p qInt64) and all pass with it. FullClickHouse.Driver.Testssuite onnet10.0after merging the current base: 9950 passed, 0 failed, 142 skipped — no existing test changed.Pre-PR validation gate
select cast(tuple(toInt64(1), 'a') as Tuple(p qInt64, r String))throws on the base branch)TestCaseparametrization, three-part test names, CHANGELOG + RELEASENOTES updated, no public API change — all touched members are internal)