ClickHouse / ClickHouse/ClickHouse

packed-io does not convert the statistics representation between full and packed parts

Open
#112,598 0 comments 0 reactions 0 assignees View on GitHub
comp-disk-abstractions
Dominant language
C++
Stars
49.9k
Forks
9k
Avg merge
21h 32m
Merged PRs (30d)
515

Description

### Describe the bug

`clickhouse disks packed-io create` / `extract` do not convert the on-disk representation of column statistics between full and packed part storage, so a converted part does not match the layout a server would have written, and in the `create` direction the resulting packed part cannot read its statistics at all.

`MergedBlockOutputStream::writeSuffixAndFinalizePart` (and `MutateTask`) pick the representation from the part's storage type (`src/Storages/MergeTree/MergedBlockOutputStream.cpp:403-416`):

* **full** part storage → one top-level file `statistics.packed`, itself a packed archive whose members are `statistics_.stats` (`serializeStatisticsPacked`);
* **packed** part storage → separate `statistics_.stats` files, which then become members of `data.packed` (`serializeStatisticsWide`, "to avoid double buffering").

`packed-io` is representation-agnostic: `createPacked` copies every top-level file of the input directory verbatim into the archive (`src/IO/PackedFilesOperations.cpp:188-225`), and `extractPacked` copies every member out verbatim. Therefore:

* `packed-io create` on a full part produces a `data.packed` whose member is `statistics.packed` (a nested archive), while a natively written packed part has `statistics_.stats` members;
* `packed-io extract` on a packed part produces a directory with top-level `statistics_.stats` files, while a natively written full part has `statistics.packed`.

The `extract` direction still reads fine, because `IMergeTreeDataPart::loadStatistics` dispatches on `checksums.has("statistics.packed")` and falls back to `loadStatisticsWide` (`src/Storages/MergeTree/IMergeTreeDataPart.cpp:1296-1315`). The `create` direction does not: `checksums` contains `statistics.packed`, so `getStatisticsPackedReader` is used, and it opens `/statistics.packed` **directly on the disk** (`IMergeTreeDataPart.cpp:1172-1195`) — a path that does not exist in a packed part, where the file is a member of `data.packed`.

### How to reproduce

Any part carrying statistics is affected. Statistics are on by default (`auto_statistics_types = 'basic, uniq_v2'`), so it is enough to have a part that went through a merge, an `ALTER TABLE ... MATERIALIZE STATISTICS`, or an explicit `STATISTICS(...)` column declaration:

```sql
CREATE TABLE t_full (id UInt64, s String) ENGINE = MergeTree ORDER BY id SETTINGS min_bytes_for_full_part_storage = 0;
INSERT INTO t_full SELECT number, randomPrintableASCII(10) FROM numbers(100);
INSERT INTO t_full SELECT number, randomPrintableASCII(10) FROM numbers(100);
OPTIMIZE TABLE t_full FINAL; -- the merged part now has statistics.packed
```

```
clickhouse disks --disk local --query "packed-io create /tmp/out/data.packed"
```

The produced archive holds `statistics.packed` as an opaque member instead of the `statistics_id.stats` / `statistics_s.stats` members a natively written packed part would have, and attaching it yields a part whose statistics cannot be read.

The same discrepancy is what makes `tests/integration/test_packed_io/test.py` diff-compare a `packed-io`-converted part against a natively written one; the test currently avoids it by creating its tables with `auto_statistics_types = ''`.

### Expected behavior

`packed-io create` and `packed-io extract` should canonicalize statistics to the representation the target storage type uses — unpacking `statistics.packed` into `statistics_.stats` members when creating a packed part, and packing them back into a single `statistics.packed` when extracting a full part — updating the checksums accordingly, so that a converted part is byte-identical to a natively written one and its statistics remain readable. Until then, `tests/integration/test_packed_io` cannot cover parts that carry statistics.

Related: https://github.com/ClickHouse/ClickHouse/pull/109454

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.