ClickHouse / ClickHouse/clickhouse-rs

RowCursor silently returns zero rows when every column is zero-width (Tuple())

Open
#459 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
559
Forks
172
Avg merge
1d 19h
Merged PRs (30d)
3

Description

### Description

`RowCursor::poll_next` (`src/cursors/row.rs:124-168`) infers "no more rows" from "no more bytes": row deserialization is only attempted while `bytes.remaining() > 0`, and when the raw cursor is exhausted with an empty buffer it returns `Ok(None)`.

`Tuple()` occupies **zero bytes** in RowBinary. A result set in which *every* column is zero-width therefore produces zero bytes of row data, and the buffer-emptiness check cannot distinguish N such rows from 0 rows. Every row is dropped **silently** — no error, no warning; `fetch_all` returns an empty `Vec` and `fetch_one` returns `Error::RowNotFound`.

This is silent under-reporting: the caller cannot detect that data was lost. `returned_rows()` also reports 0.

Contrast case (works correctly today, must not regress): as soon as one non-zero-width column is present, rows have a non-zero byte length and the cursor is correct — `SELECT tuple(), 5 :: UInt8 FROM numbers(3)` returns 3 rows.

Server-side evidence:

```
$ curl -s 'http://localhost:8123/?query=SELECT tuple() FROM numbers(3) FORMAT RowBinary' | wc -c
0 # 3 rows, 0 bytes of row data

$ curl -s 'http://localhost:8123/?query=SELECT count() FROM (SELECT tuple() FROM numbers(3))'
3
```

The row count genuinely cannot be recovered from the RowBinary byte stream here — the server's output for 3 zero-width rows is byte-identical to its output for 0 rows.

**Secondary, independent defect found while reproducing this:** with validation enabled (the default), the query fails earlier in the `RowBinaryWithNamesAndTypes` header parser, because `clickhouse-types` rejects `Tuple()`:

```
Err(InvalidColumnsHeader(TypeParsingError("Invalid Tuple format, expected Tuple(Type1, Type2, ...), got Tuple()")))
```

That comes from `parse_tuple` in `types/src/data_types.rs:166`, which requires at least one inner type even though `DataTypeNode::Tuple(vec![])` exists and round-trips to `"Tuple()"` (`types/src/data_types.rs:962`). So on `main` the zero-row bug is only observable with `with_validation(false)`; with validation on the user gets a spurious parse error instead. Both need fixing.

### ClickHouse server version

26.7.3.19 (verified against a running server, not by inspection).

### Reproduction

`tests/it/zerowidth.rs` (registered as `mod zerowidth;` in `tests/it/main.rs`):

```rust
use clickhouse::Row;
use serde::{Deserialize, Serialize};

#[derive(Debug, Row, Serialize, Deserialize)]
struct EmptyTupleRow {
t: (),
}

#[derive(Debug, Row, Serialize, Deserialize)]
struct MixedRow {
t: (),
n: u8,
}

#[tokio::test]
async fn all_zero_width_columns() {
// validation disabled to work around the `Tuple()` header-parser defect described above
let client = crate::get_client().with_validation(false);

let rows = client
.query("SELECT tuple() AS t FROM numbers(3)")
.fetch_all::()
.await;
println!("all zero-width: {rows:?}");
assert_eq!(rows.unwrap().len(), 3, "server sent 3 rows");
}

#[tokio::test]
async fn mixed_width_columns() {
let client = crate::get_client().with_validation(false);

let rows = client
.query("SELECT tuple() AS t, 5 :: UInt8 AS n FROM numbers(3)")
.fetch_all::()
.await;
println!("mixed: {rows:?}");
assert_eq!(rows.unwrap().len(), 3);
}
```

Actual output:

```
mixed: Ok([MixedRow { t: (), n: 5 }, MixedRow { t: (), n: 5 }, MixedRow { t: (), n: 5 }])
test zerowidth::mixed_width_columns ... ok

all zero-width: Ok([])
thread 'zerowidth::all_zero_width_columns' panicked at tests/it/zerowidth.rs:24:5:
assertion `left == right` failed: server sent 3 rows
left: 0
right: 3
test zerowidth::all_zero_width_columns ... FAILED
```

Expected: both return 3 rows.

With the default `get_client()` (validation on), both tests instead fail with the `Tuple()` parse error quoted above.

### Suggested fix

Row boundaries have to come from somewhere other than byte presence. Options, roughly by intrusiveness (maintainer design decision):

1. Detect the degenerate case: when every parsed column type has a fixed zero wire width, the byte stream carries no row information, so source the count out of band — the `X-ClickHouse-Summary` header is already parsed and exposed (`RowCursor::summary()`), or a wrapping query. Narrow blast radius: no change to any normal result set. Requires validation (the columns header) to be on, so it pairs with fixing `parse_tuple`.
2. Reject rather than silently truncate: if the count cannot be established, return an error for an all-zero-width result set instead of reporting zero rows. Worse for users than (1) but far better than silent data loss.
3. Read such result sets in a format that frames rows explicitly (e.g. `Native` block headers carry the row count — see `src/cursors/native.rs`). Largest change.

Separately, `parse_tuple` (`types/src/data_types.rs:166`) should accept `Tuple()` and produce `DataTypeNode::Tuple(vec![])`.

### Link

Same root cause reported for the .NET client: https://github.com/ClickHouse/clickhouse-cs/issues/570

Contributor guide

Open the contributing guide

Research direction

Start with RowCursor::poll_next in src/cursors/row.rs, parse_tuple in types/src/data_types.rs, and the reproduction in tests/it/zerowidth.rs. Run the zero-width integration tests and inspect the existing summary and native cursor handling before choosing the maintainer-approved row-count strategy. Done means Tuple() parses successfully, zero-width rows are handled without silent loss, and mixed-width behavior still passes.

Written by the indexing model from the issue text.

Assessment

Tech stack
clickhouse, rust
Domain
database
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.