ClickHouse / ClickHouse/ClickHouse
createLightweightDeleteCommand does not copy mutation_version from source command
- Dominant language
- C++
- Stars
- 49.9k
- Forks
- 9k
- Avg merge
- 21h 32m
- Merged PRs (30d)
- 515
Description
_Found via ClickGap automated review. Please close or comment if this is incorrect or needs adjustment._
_Retrospective finding from a historical scan of [PR #79281](https://github.com/ClickHouse/ClickHouse/pull/79281) (merged 2025-05-13). Confirmed on current codebase — close with a note if already fixed._
### Describe what's wrong
When a lightweight delete (UPDATE _row_exists=0) is converted to a DELETE command by createLightweightDeleteCommand, the mutation_version is lost (defaults to nullopt). This causes perform_alter_conversions to be incorrectly computed as false when combined with ALTER MODIFY COLUMN, because nullopt > version_of_alter_mutation always returns false.
**Root cause:** AlterConversions.cpp:102 - MutationCommand::parse(*alter_command) does not set mutation_version (it's not stored in ASTAlterCommand). The function should copy command.mutation_version to the result, like createCommandWithUpdatedColumns does at line 59.
**Why we believe this is a bug:** AlterConversions.cpp:90-108 createLightweightDeleteCommand() → MutationCommand::parse() → returns command with mutation_version=nullopt. Compare with createCommandWithUpdatedColumns() at line 59 which correctly copies mutation_version. At line 305, perform_alter_conversions = !version_of_alter_mutation || actions.mutation_version > version_of_alter_mutation evaluates incorrectly when mutation_version is nullopt.
**Affected locations:**
- `src/Storages/MergeTree/AlterConversions.cpp:102` — createLightweightDeleteCommand: mutation_version not copied
- `src/Storages/MergeTree/AlterConversions.cpp:305` — perform_alter_conversions uses mutation_version for comparison
**Impact:** When lightweight DELETE + ALTER MODIFY COLUMN are both pending with apply_mutations_on_fly, the DELETE step will always have perform_alter_conversions=false regardless of mutation ordering, causing LOGICAL_ERROR crash (Bad cast between column types). Currently masked by a broader pre-existing issue where ANY DELETE + ALTER MODIFY + on-fly crashes.
### Does it reproduce on most recent release?
Yes — confirmed on current `master` (commit `11ce418c0505`).
### How to reproduce
```sql
-- Test: lightweight DELETE + ALTER MODIFY COLUMN with apply_mutations_on_fly
-- Covers: AlterConversions.cpp - DELETE mutation combined with ALTER MODIFY COLUMN on fly
DROP TABLE IF EXISTS t_lwd_modify_fly;
CREATE TABLE t_lwd_modify_fly (a UInt32, b Int32) ENGINE = MergeTree ORDER BY a;
INSERT INTO t_lwd_modify_fly VALUES (1, 10) (2, 20) (3, 30);
-- Async lightweight delete
DELETE FROM t_lwd_modify_fly WHERE a = 1 SETTINGS lightweight_deletes_sync = 0;
-- Async ALTER MODIFY COLUMN (type change)
ALTER TABLE t_lwd_modify_fly MODIFY COLUMN b Int64 SETTINGS mutations_sync = 0, alter_sync = 0;
SYSTEM STOP MERGES t_lwd_modify_fly;
-- This should return rows 2,3 with correct Int64 type
-- BUG: crashes with LOGICAL_ERROR: Bad cast from ColumnVector to ColumnVector
SELECT a, b FROM t_lwd_modify_fly ORDER BY a SETTINGS apply_mutations_on_fly = 1;
DROP TABLE t_lwd_modify_fly;
```
### Expected behavior
```
2 20
3 30
```
### Error message and/or stacktrace
```
Code: 49. DB::Exception: Bad cast from type DB::ColumnVector to DB::ColumnVector. (LOGICAL_ERROR)
```
### Additional context
**Open risks:**
- The broader pre-existing crash (DELETE + ALTER MODIFY + on-fly) masks this specific bug - even with the fix, the combination may still crash due to the general issue
**Suggested fix:** After line 102 in createLightweightDeleteCommand, add: mutation_command->mutation_version = command.mutation_version; (matching the pattern at line 59 in createCommandWithUpdatedColumns)
**Analysis details:** Confidence HIGH | Severity P1 | Testability: `STATELESS_SQL`
Found during automated review of [PR #79281](https://github.com/ClickHouse/ClickHouse/pull/79281).
---
_ClickGapAI · Confidence: HIGH · Severity: P1 · Finding: `h_pr79281_001`_
Contributor guide
Assessment
This issue has not been assessed yet.