ClickHouse / ClickHouse/ClickHouse
Missing metadata_version.txt silently loses column data on ATTACH of a part detached before a metadata-only ALTER (corrects "missing file is safe" in #116481)
- Dominant language
- C++
- Stars
- 49.9k
- Forks
- 9k
- Avg merge
- 21h 32m
- Merged PRs (30d)
- 515
Description
### Describe what's wrong
A part that was detached before a metadata-only schema change (e.g. `RENAME COLUMN`, `MODIFY COLUMN`) and that is missing its `metadata_version.txt` file is silently loaded at the table's *current* metadata version on `ATTACH`, instead of at its own (older) on-disk version. The pending `alter_conversions` that the part still needs are therefore skipped, and `ATTACH` succeeds with no error while serving default/wrong values for the renamed or modified columns — silent data loss.
This directly contradicts the "a missing file uses the safe fallback" conclusion in #116481. That conclusion is correct only when the part's data already matches the current schema. When the part was detached before a schema change, the "safe fallback" to the current version is wrong: the fallback tells the read path the part is already up-to-date when it is not. The load path itself documents exactly this hazard in `MergeTreeData::loadPartAndFixMetadataImpl` (`src/Storages/MergeTree/MergeTreeData.cpp`):
```cpp
/// Do not overwrite metadata version from the table:
/// the part may have been detached before schema changes (e.g. RENAME COLUMN),
/// and its metadata_version should reflect which schema changes have actually been applied
/// to the part's data. Overwriting it would make the mutation/rename system think
/// the part is already up-to-date when it's not.
```
The on-disk-version-preserving code honors that comment only when `metadata_version.txt` is present. When the file is absent, `IMergeTreeDataPart::loadColumns` falls into the fallback that does the very thing the comment warns against:
```cpp
if (!loaded_metadata_version)
{
auto storage_metdata_snapshot = storage.getInMemoryMetadataPtr(storage.getContext(), false);
loaded_metadata_version = storage_metdata_snapshot->getMetadataVersion(); // == current table version
old_part_with_no_metadata_version_on_disk = true;
}
```
`old_part_with_no_metadata_version_on_disk` only relaxes a mutation column-drop check (`MutateTask.cpp`); it does not cause the skipped conversions to be re-applied.
`metadata_version.txt` is not included in the part checksums (`getFileNamesWithoutChecksums`), so its absence is never detected by `CHECK TABLE` or by consistency validation. A missing file is a reachable crash artifact: as shown in #116628, `DETACH PARTITION`/detach clones the part into `detached/` under its final name by hard-linking files one at a time in directory-entry order with no temporary prefix and no atomic rename, so a crash mid-clone can leave this single not-checksummed file absent while the rest of the part is intact. The reproducer below models that on-disk state by removing the file, exactly as the control in #116481 does.
Impact: silent wrong results / data loss on plain `MergeTree` and `ReplicatedMergeTree` for any column touched by a lazy metadata-only `ALTER` that the detached part had not yet applied. `SharedMergeTree` stores the version in Keeper rather than in this file and is not affected.
### Does it reproduce on the most recent release?
Yes, reproduces on 26.9.
### How to reproduce
Single node, default settings.
```sql
CREATE TABLE t (id UInt64, a UInt32)
ENGINE = MergeTree ORDER BY id
SETTINGS min_bytes_for_wide_part = 0;
INSERT INTO t SELECT number, number FROM numbers(1000);
SELECT count(), sum(a) FROM t;
-- 1000 499500 (acknowledged and durable to the client)
-- Detach the part BEFORE the schema change, then change the schema.
ALTER TABLE t DETACH PARTITION tuple();
ALTER TABLE t RENAME COLUMN a TO b; -- table metadata version -> 1; detached part stays at 0
```
The detached clone still has `metadata_version.txt` = 0 (its true, pre-rename version). Simulate the crash-torn clone by removing that single file from the detached directory:
```
rm store//all_1_1_0/metadata_version.txt
```
```sql
ALTER TABLE t ATTACH PARTITION tuple(); -- succeeds, no error
SELECT count(), sum(b) FROM t;
-- 1000 0 (all 1000 rows' renamed-column values silently replaced by defaults)
```
Control that isolates the file — repeat with `metadata_version.txt` left intact:
```sql
ALTER TABLE t ATTACH PARTITION tuple();
SELECT count(), sum(b) FROM t;
-- 1000 499500 (part loads at version 0, the b<-a conversion is applied, data intact)
```
So a detached part with `metadata_version.txt` present is read correctly, while the same part with the file missing silently loses the column data — the opposite of the "missing file is safe" behavior reported in #116481, whose test kept the part attached with data already matching the current schema. The same result holds on `ReplicatedMergeTree` (single replica, or before the part has replicated).
### Expected behavior
`ATTACH` of a part missing `metadata_version.txt` should not silently assume the part is already at the current schema. Either the part's real metadata version must be recovered/validated before applying the current-version fallback, or `metadata_version.txt` should be checksummed so its absence on a modern part is detected and the part is quarantined rather than served with skipped conversions. A part that has not applied a pending `RENAME COLUMN`/`MODIFY COLUMN` must have those conversions applied on read, exactly as it does when the file is present.
### Error message and/or stacktrace
No error is produced — `ATTACH` succeeds and `SELECT` returns wrong values silently. `CHECK TABLE` reports the part and table as OK.
### Additional context
Related: #116481 (zero-length `metadata_version.txt` is fatal; this report shows the "missing file is safe" companion claim is not true for a part detached before a schema change) and #116628 (crash during detach leaves a partial clone in `detached/` under a valid name — the mechanism by which `metadata_version.txt` can go missing while the rest of the part survives). Fix direction is the same family as #116481: give `metadata_version.txt` checksum protection, or make the missing-file path fail-closed for parts whose data version is behind the table.
Contributor guide
Assessment
This issue has not been assessed yet.