-
Notifications
You must be signed in to change notification settings - Fork 622
[r2dbc] getRowsUpdated() returns 0 for successful INSERT...SELECT — needs reliable written_rows #2860
Description
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_insertis a no-op forINSERT...SELECT, but we set it
globally for theINSERT VALUESpath on the same connection)
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:
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):
Mono<? extends UpdateCount> 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:
- Source
getRowsUpdated()fromSummary.getStatistics()(or whichever
field is authoritative post-completion) instead of fromSummary.getProgress(). - Expose the raw
ClickHouseResponseSummaryfromClickHouseResult(or a
similar handle), so callers can read the final fields themselves. - Document the current semantics so applications know not to rely on
getRowsUpdated()forINSERT...SELECT.
Happy to send a PR for (1) or (2) — please confirm which direction you'd
prefer.
Environment
clickhouse-r2dbc: 0.9.0clickhouse-client/clickhouse-http-client: 0.9.0- ClickHouse server: 25.3.x
- Connection: HTTP transport