ClickHouse / ClickHouse/clickhouse-java

[r2dbc] getRowsUpdated() returns 0 for successful INSERT…SELECT — needs reliable written_rows

Abierto
#2,860 6 comentarios 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
Java
Estrellas
1.6k
Forks
636
Merge medio
2 d 23 h
PR fusionados (30 d)
29

Descripción

## Summary

`ClickHouseResult.getRowsUpdated()` in `clickhouse-r2dbc` returns 0 for
successful `INSERT INTO … SELECT FROM …` queries that did write rows. This
makes it impossible to reliably distinguish "INSERT…SELECT wrote 0 rows
because the SELECT produced none" from "INSERT…SELECT wrote N rows but the
driver reported 0" at the application layer.

The same row count appears correctly in `system.query_log.written_rows`
server-side, and the HTTP `X-ClickHouse-Summary` header also carries an
accurate `written_rows` once the query finishes. The information exists; the
driver just doesn't expose it via the standard R2DBC `Result.getRowsUpdated()`
contract for this query shape.

## Reproduction

- Driver: `com.clickhouse:clickhouse-r2dbc:0.9.0` (also reproduces on 0.8.x)
- Server: ClickHouse 25.3
- Query shape: `INSERT INTO target_table (...) SELECT ... FROM source_table WHERE ...`
- Connection settings: `async_insert=1, wait_for_async_insert=1`
(per docs, `async_insert` is a no-op for `INSERT…SELECT`, but we set it
globally for the `INSERT VALUES` path on the same connection)

```java
Flux.from(statement.execute())
.flatMap(Result::getRowsUpdated) // emits 0 even when N rows were inserted
.reduce(0L, Long::sum)
// observed: returns 0
```

Verifying server-side after the query finishes:

```sql
SELECT written_rows
FROM system.query_log
WHERE query_id = '...' AND type = 'QueryFinish';
-- returns N (the correct count)
```

## Root cause

Looking at `ClickHouseResult` constructor (current `main`):

```java
Mono updatedCount = Mono.just(response)
.map(ClickHouseResponse::getSummary)
.map(ClickHouseResponseSummary::getProgress)
.map(ClickHouseResponseSummary.Progress::getWrittenRows)
.map(UpdateCount::new);
```

The driver reads `written_rows` from `Summary.getProgress()`, which is the
**interim progress event** snapshot — not the final summary. For
`INSERT…SELECT` queries, a definitive post-completion written_rows is
typically reflected in `Summary.getStatistics()` (or in a final progress
event that doesn't always land before the subscriber observes completion).

`ClickHouseResponseSummary` exposes both `getProgress()` and `getStatistics()`,
and the top-level `getWrittenRows()` delegates to progress.

## Use case

Detecting at the application layer when an `INSERT…SELECT` wrote zero rows
(to surface inconsistency conditions before committing dependent state).
Since `getRowsUpdated()` can return 0 even on success, the check fires
false positives.

## Asks

Any one of the following would unblock us:

1. Source `getRowsUpdated()` from `Summary.getStatistics()` (or whichever
field is authoritative post-completion) instead of from `Summary.getProgress()`.
2. Expose the raw `ClickHouseResponseSummary` from `ClickHouseResult` (or a
similar handle), so callers can read the final fields themselves.
3. Document the current semantics so applications know not to rely on
`getRowsUpdated()` for `INSERT…SELECT`.

Happy to send a PR for (1) or (2) — please confirm which direction you'd
prefer.

## Environment

- `clickhouse-r2dbc`: 0.9.0
- `clickhouse-client` / `clickhouse-http-client`: 0.9.0
- ClickHouse server: 25.3.x
- Connection: HTTP transport

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.