ClickHouse / ClickHouse/clickhouse-java

client-v2: geo columns (Point/Ring/Polygon/...) are misread in the Native format

Open
#3,088 0 comments 0 reactions 0 assignees View on GitHub
area:data-type bug client-api-v2
Dominant language
Java
Stars
1.6k
Forks
636
Avg merge
2d 16h
Merged PRs (30d)
28

Description

## Description

`client-v2` cannot read geo columns (`Point`, `Ring`, `LineString`, `MultiLineString`, `Polygon`, `MultiPolygon`) from the **Native** format. The same queries read correctly with `RowBinaryWithNamesAndTypes`.

Two symptoms, by type:

* **`Point` with more than one row in a block — silent data corruption.** The coordinates are returned scrambled across rows. No exception, and a fixed-width column after the geo column still reads correctly, so nothing signals the corruption.
* **`Ring` / `LineString` / `MultiLineString` / `Polygon` / `MultiPolygon` — the block desynchronizes** and the read fails with `IllegalArgumentException: Non-empty typeName is required`, which does not indicate the cause.

`Point` with exactly one row per block reads correctly by coincidence (with one row, the columnar and the row-wise layouts are byte-identical).

### Steps to reproduce

1. Run a query that selects a geo column with `QuerySettings.setFormat(ClickHouseFormat.Native)` and read it with `client.newBinaryFormatReader(response)`.
2. For `Point`, use a query that returns 3 rows in one block; the returned coordinates do not match the server.
3. For `Ring` (or any other multi-point geo type), the read throws.

### Error Log or Exception StackTrace

```
java.lang.IllegalArgumentException: Non-empty typeName is required
at com.clickhouse.data.ClickHouseColumn.of(ClickHouseColumn.java:...)
at com.clickhouse.client.api.data_formats.NativeFormatReader.readBlock(NativeFormatReader.java:87)
```

The exception is a consequence of the desync: after the geo column is read with the wrong layout, the stream position is inside the payload, so the *next* column header is parsed as an empty name/type.

### Expected Behaviour

The Native format must return the same values as `RowBinaryWithNamesAndTypes`, which agrees with the server.

Server (ClickHouse 26.7.3.19), `FORMAT JSONCompactEachRow`:

```
[0, [1,2], 42]
[1, [3,4], 42]
[2, [5,6], 42]
```

`RowBinaryWithNamesAndTypes` (correct):

```
row 0: rowId=0 g=[1.0, 2.0] tail=42
row 1: rowId=1 g=[3.0, 4.0] tail=42
row 2: rowId=2 g=[5.0, 6.0] tail=42
```

`Native` (actual — coordinates scrambled, no error):

```
row 0: rowId=0 g=[1.0, 3.0] tail=42
row 1: rowId=1 g=[5.0, 2.0] tail=42
row 2: rowId=2 g=[4.0, 6.0] tail=42
```

`Ring`, same shape, `Native` (actual):

```
EXCEPTION: java.lang.IllegalArgumentException: Non-empty typeName is required
```

while `RowBinaryWithNamesAndTypes` returns the correct `[[1.0,2.0],[3.0,4.0]]`, `[[11.0,12.0],[13.0,14.0]]`, `[[21.0,22.0],[23.0,24.0]]`.

### Root cause

`NativeFormatReader.readBlock()` (`client-v2/src/main/java/com/clickhouse/client/api/data_formats/NativeFormatReader.java:109`) enters its columnar branch only when `column.isArray()` is true. The concrete geo types are their own `ClickHouseDataType` values, so `isArray()` is false for all of them and they fall through to the `else` branch at line 118, which calls `binaryStreamReader.readValue(column)` once per row. That dispatches to the **RowBinary** geo decoders — `readGeoPoint()` (`BinaryStreamReader.java:1123`), `readGeoRing()` (`:1132`), `readGeoPolygon()` (`:1147`), `readGeoMultiPolygon()` (`:1161`).

The two encodings are not interchangeable:

* `Point` is `Tuple(Float64, Float64)`. Native writes it column-major — all x values, then all y values. `readGeoPoint()` reads two *adjacent* doubles as one point. The byte count per block is the same either way, so the column boundary is preserved and the error stays silent; only the pairing is wrong.
* `Ring` / `LineString` are `Array(Point)`. Native writes cumulative UInt64 offsets, then the element tuple column-major. `readGeoRing()` reads a per-row varuint count followed by interleaved `(x, y)` pairs, so the first offset byte is consumed as a point count and everything after that is misaligned. `Polygon` / `MultiPolygon` add further offset levels with the same result.

Native bytes for the 3-row `Point` query above, showing the column-major layout the reader does not expect:

```
0303 05 726f774964 06 55496e743634 3 cols, 3 rows, 'rowId' UInt64
0000000000000000 0100000000000000 0200000000000000 0, 1, 2
01 67 05 506f696e74 'g' Point
000000000000f03f 0000000000000840 0000000000001440 x column: 1, 3, 5
0000000000000040 0000000000001040 0000000000001840 y column: 2, 4, 6
04 7461696c 05 496e743332 2a000000 2a000000 2a000000 'tail' Int32: 42, 42, 42
```

### Suggested fix

Two options; the second is smaller but is itself a behavior change:

1. **Decode geo columns column-major in the Native reader.** Route them into the existing columnar path by treating each as its array-of-tuple equivalent (`Point` → columnar `Tuple(Float64, Float64)`, `Ring`/`LineString`/`MultiPoint` → `Array(Point)`, `Polygon`/`MultiLineString` → `Array(Ring)`, `MultiPolygon` → `Array(Polygon)`), then assemble the `double[]` / `double[][]` / `double[][][]` / `double[][][][]` values the RowBinary decoders return today, so the value shape returned to callers does not change.
2. **Reject geo columns in the Native format** with a clear `ClientException` pointing at `RowBinaryWithNamesAndTypes`, matching the precedent already in `readBlock` for the QBit shapes it does not decode (`NativeFormatReader.java:102`). This turns silent corruption into a loud, actionable failure, but it removes a read path that currently appears to work for single-row `Point`.

Whichever is chosen, a regression test should place the geo column **in the middle of the schema with a fixed-width column after it** and read **several rows in one block** — a single-row `Point` passes even with the current code.

Two contrast cases must keep their current behavior: reading these types with `RowBinaryWithNamesAndTypes` is correct today, and the geo **write** path is unaffected.

### Related

* #2955 / #2956 fix per-row lengths from cumulative offsets *inside* the `isArray()` branch. Geo columns never reach that branch, so that fix does not cover them.
* Surfaced by a review comment on #3050 (MultiPoint support): https://github.com/ClickHouse/clickhouse-java/pull/3050#discussion_r3894055614. This defect is pre-existing and independent of that PR, whose only `BinaryStreamReader` change is one extra `case MultiPoint:` in the RowBinary switch. `MultiPoint` shares the `Array(Point)` layout, so once merged it behaves like `Ring` here.

### Code Example

```java
Client client = new Client.Builder()
.addEndpoint("http://localhost:8123")
.setUsername("default").setPassword("")
.compressServerResponse(false)
.build();

String sql = "SELECT number AS rowId,"
+ " (toFloat64(number*2+1), toFloat64(number*2+2))::Point AS g,"
+ " toInt32(42) AS tail FROM numbers(3) ORDER BY rowId";

QuerySettings settings = new QuerySettings().setFormat(ClickHouseFormat.Native);
try (QueryResponse response = client.query(sql, settings).get()) {
ClickHouseBinaryFormatReader reader = client.newBinaryFormatReader(response);
while (reader.next() != null) {
System.out.println(Arrays.toString((double[]) reader.readValue("g")));
}
}
// prints [1.0, 3.0] / [5.0, 2.0] / [4.0, 6.0]
// expected [1.0, 2.0] / [3.0, 4.0] / [5.0, 6.0]

// Replacing Point with ::Ring and the same 3-row shape throws instead:
// java.lang.IllegalArgumentException: Non-empty typeName is required
```

Affected geo types, verified one by one against the server with both formats:

| Type | RowBinaryWithNamesAndTypes | Native |
| --- | --- | --- |
| `Point`, 1 row/block | correct | correct (layouts coincide) |
| `Point`, 3 rows/block | correct | **wrong values, no error** |
| `Ring` | correct | throws |
| `LineString` | correct | throws |
| `MultiLineString` | correct | throws |
| `Polygon` | correct | throws |
| `MultiPolygon` | correct | throws |

### Configuration

#### Environment
* [ ] Cloud
* Client version: `0.11.0-rc1` (`main` at 0b781da1f)
* Language version: OpenJDK 17.0.18
* OS: Ubuntu 24.04 (container)

#### ClickHouse Server
* ClickHouse Server version: 26.7.3.19
* ClickHouse Server non-default settings, if any: none
* `CREATE TABLE` statements for tables involved: none — reproduces on a `SELECT` from `numbers()`
* Sample data for all these tables: n/a

---

Found by automated analysis of the client while working on #3050, and verified against a live ClickHouse server (26.7.3.19) rather than by code inspection: every row above was produced by running both formats through `client.newBinaryFormatReader(...)` and comparing with the server's own `JSONCompactEachRow` output.

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.