ClickHouse / ClickHouse/clickhouse-java
[client-v2] ClickHouseColumn mis-parses a Tuple whose JSON element is not last: later elements are swallowed as JSON parameters
- Dominant language
- Java
- Stars
- 1.6k
- Forks
- 636
- Avg merge
- 2d 16h
- Merged PRs (30d)
- 28
Description
### Summary
`ClickHouseColumn.of(...)` mis-parses a `Tuple` whose `JSON` element is not the last one: every element after the `JSON` up to the next closing parenthesis is swallowed as the JSON column's parameter list. Depending on what follows, the Tuple either silently loses elements or the parse throws `Unknown data type`.
Reproduced on `clickhouse-data` **0.9.5** and **0.10.0** (latest published), OpenJDK 17. No server needed.
### Reproduce
```java
import com.clickhouse.data.ClickHouseColumn;
ClickHouseColumn c = ClickHouseColumn.of("x", "Tuple(JSON, FixedString(3))");
c.getNestedColumns().size(); // 1 (expected 2)
c.getNestedColumns().get(0).getOriginalTypeName(); // "JSON, FixedString(3)"
ClickHouseColumn.of("x", "Tuple(JSON, Decimal(10, 2))");
// java.lang.IllegalArgumentException: Unknown data type: 2
```
Output of the snippet above (jshell, same on both versions):
```
Tuple(JSON, FixedString(3)) -> nested=1 first="JSON, FixedString(3)"
Tuple(JSON, Decimal(10, 2)) -> java.lang.IllegalArgumentException: Unknown data type: 2
Tuple(JSON, Int32) -> nested=2 (ok: nothing with parentheses after JSON)
Tuple(Int32, JSON) -> nested=2 (ok: JSON is last)
Tuple(FixedString(3), JSON) -> nested=2 (ok)
JSON(max_dynamic_paths=10) -> [max_dynamic_paths=10] (ok: the '(' is adjacent)
```
So the trigger is a non-final `JSON` element followed, anywhere later in the same Tuple, by a type that has parentheses.
### Root cause
`ClickHouseColumn.readColumn`, 0.9.5 lines 508-516 (0.10.0: from line 553, unchanged):
```java
} else if (args.startsWith(KEYWORD_JSON, i)) {
int index = args.indexOf('(', i + KEYWORD_JSON.length()); // scans to the END of the type string
if (index > i) {
i = ClickHouseUtils.skipBrackets(args, index, len, '(');
String originalTypeName = args.substring(startIndex, i);
...
parseJSONColumn(args.substring(index + 1, i - 1), nestedColumns, parameters);
```
`indexOf('(')` starts after the keyword but is not bounded to the next character, so inside a Tuple the `(` of a *later* element is found. `skipBrackets` then consumes through that element's matching `)`, the span `JSON, FixedString(3)` becomes one JSON column, and `3` is fed to `parseJSONColumn` as its parameters. The enclosing Tuple parser resumes after the swallowed text, sees its own `)`, and ends with one element fewer.
- With `FixedString(3)`: `parseJSONColumn("3")` reads `3` as a path name, finds no type, and returns silently, so the Tuple is short with no error.
- With `Decimal(10, 2)`: `parseJSONColumn("10, 2")` reads `10` as a path name and tries to parse `2` as its type: `Unknown data type: 2`.
### Impact
`Client.getTableSchema()` fails for any table containing such a column (second case), or returns a Tuple with fewer nested columns than the type text declares (first case). A writer that serializes from the parsed columns but sends `getOriginalTypeName()` in a `RowBinaryWithNamesAndTypes` header then writes short rows against a header that promises more, which corrupts or fails the batch with no indication of the cause.
### Suggested fix
Treat `(` as the JSON parameter list only when it immediately follows the keyword, e.g. `index == i + KEYWORD_JSON.length()` (optionally after whitespace), matching the generic branch below it, which only calls `readParameters` when the *current* character is `(`.
Contributor guide
Assessment
This issue has not been assessed yet.