Skip to content

fix(types): parse back-quoted JSON path names - #458

Merged
abonander merged 1 commit into
ClickHouse:mainfrom
teddytennant:fix/json-hint-quoted-paths
Aug 25, 2026
Merged

abonander merged 1 commit into
ClickHouse:mainfrom
teddytennant:fix/json-hint-quoted-paths

Conversation

@teddytennant

Copy link
Copy Markdown
Contributor

Summary

Fixes #453.

ClickHouse back-quotes a JSON typed path when its name contains a character that requires quoting, and reports the column type that way in the RowBinaryWithNamesAndTypes header:

$ curl -s --data-binary "SELECT toTypeName('{\"a b\":1}'::JSON(\`a b\` Int64)) AS t, toTypeName('{}'::JSON(\`a,b\` Int64)) AS t2 FORMAT TSV" http://localhost:8123/
JSON(`a b` Int64)	JSON(`a,b` Int64)

parse_json() split the hint list on every comma and each hint on every space, with no awareness of those quotes:

  • JSON(`a,b` Int64) was split into `a and b` Int64. The first fragment contains no space, so map had a single element and map[1] panicked with index out of bounds: the len is 1 but the index is 1 — a panic in library code rather than a returned Err.
  • JSON(`a b` Int64) yielded the key `a and the type b`, failing with `Unknown data type: b``.

Both happen while parsing the columns header, so the whole query fails — reading or inserting into any table with such a column is impossible, and there is no way to opt out.

The fix

Splitting is now done on separators that are outside a back-quoted span, both for the hint list and for the path/type boundary inside a hint (a back-quote escaped with a backslash does not end the span, matching how the server escapes it). The same applies to the = check that tells a JSON setting such as max_dynamic_types=8 apart from a path hint, so a path named `b=c` is no longer silently dropped. Where the old code indexed map[1] blindly, a hint with no type now returns a TypeParsingError.

This is the right layer for it: the quoting is part of the type as the server spells it, and parse_json() is the only place that takes it apart.

Path names are deliberately kept exactly as the server wrote them, quotes included, rather than being unquoted. DataTypeNode's Display impl is used by put_rbwnat_columns_header() to send the type back on INSERT, so keeping the name verbatim makes that round-trip byte-identical; unquoting the name would silently produce an invalid JSON(a b Int64) header unless Display learned to re-quote and re-escape. That is a separate change, so I left it out here — the unit test pins the round-trip so the two sides cannot drift apart.

Not addressed, and independent of this: #442, where remove_json_header() trims all trailing ) and so mangles a nested type argument like Nullable(String). A fix for that would not have helped the back-quoted cases and vice versa.

Verification

Unit test, on main before the fix:

$ cargo test -p clickhouse-types --lib test_data_type_new_json_with_quoted_paths

running 1 test
test data_types::tests::test_data_type_new_json_with_quoted_paths ... FAILED

---- data_types::tests::test_data_type_new_json_with_quoted_paths stdout ----

thread 'data_types::tests::test_data_type_new_json_with_quoted_paths' panicked at types/src/data_types.rs:1486:52:
called `Result::unwrap()` on an `Err` value: TypeParsingError("Unknown data type: b`")

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 42 filtered out; finished in 0.01s

The integration test, against a real server (26.7.3.19), reproduces the panic itself:

$ cargo test --test it insert_with_json_hint_and_quoted_paths

running 1 test
test insert::insert_with_json_hint_and_quoted_paths ... FAILED

---- insert::insert_with_json_hint_and_quoted_paths stdout ----
Created database chrs__insert__insert_with_json_hint_and_quoted_paths

thread 'insert::insert_with_json_hint_and_quoted_paths' panicked at types/src/data_types.rs:723:51:
index out of bounds: the len is 1 but the index is 1

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 192 filtered out; finished in 0.04s

Both pass with the fix, and the rest of the suite is unaffected:

$ cargo test -p clickhouse-types --lib
test result: ok. 43 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s

$ cargo test --workspace           # docker compose up -d
test result: ok. 71 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
test result: ok. 188 passed; 0 failed; 5 ignored; 0 measured; 0 filtered out
test result: ok. 8 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
test result: ok. 43 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

cargo fmt -- --check and cargo clippy --all-targets --no-default-features are clean with RUSTFLAGS=-Dwarnings.

Checklist

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided so that we can include it in CHANGELOG later

Suggested CHANGELOG entry (Fixed): JSON columns whose typed paths are back-quoted, e.g. JSON(`a b` Int64) or JSON(`a,b` Int64), no longer fail to parse; the comma case previously panicked with an index out of bounds. ([#453])

@CLAassistant

CLAassistant commented Aug 8, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

ClickHouse back-quotes a JSON typed path whose name contains a character
that requires quoting, and reports the column type accordingly, e.g.
JSON(`a b` Int64) or JSON(`a,b` Int64).

parse_json() split the hint list on every comma and every space, without
looking at those quotes, so such a type was torn apart: the comma case
produced a fragment with no space in it and panicked on map[1] with an
index out of bounds, and the space case produced the key `a and the type
b`, failing with "Unknown data type: b`". Both happen while parsing the
columns header, so the whole query fails.

Split the hint list, and each hint into a path and a type, only on
separators that are outside a back-quoted span, and return a parsing
error instead of indexing blindly when a hint has no type. Path names
are kept exactly as the server spelled them, including the quotes, so
that the type is serialized back unchanged in an INSERT header.
@teddytennant

Copy link
Copy Markdown
Contributor Author

CLA is signed and this is ready for review whenever someone has a moment.

@abonander
abonander merged commit ef1efc1 into ClickHouse:main Aug 25, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSON typed paths containing a space or a comma fail to parse (comma case panics with index out of bounds)

3 participants