ClickHouse / ClickHouse/clickhouse-java

[client-v2] ClickHouseColumn mis-parses a Tuple whose JSON element is not last: later elements are swallowed as JSON parameters

Open
#3,098 1 comment 0 reactions 0 assignees View on GitHub
area:data-type bug
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.