Workflow [PR], commit [e81e737]

Summary: ✅

• Performance Comparison: Performance dashboard

AI Review

Summary

This PR removes redundant projection .proj checksum insertion from MergedColumnOnlyOutputStream::fillChecksums and adds the mutation-side insertion in MutateSomePartColumnsTask::finalize. I found no unresolved correctness issues: vertical merges still add projection checksums in MergedBlockOutputStream::finalizePartAsync, column-only mutations add rebuilt projection checksums before checksums.txt is written, and the current PR CI report has no failed checks.

Final Verdict

Status: ✅ Approve

No required actions.

@groeneai, provide a fix for FileCacheTest.

@alexey-milovidov @amosbird — FileCacheTest.SLRUFreeSpaceKeepingProtectedOnly is not caused by this PR, it's a chronic flake hitting many unrelated PRs.

Cross-PR data (CIDB, last 30 days)

The same test fails on 21 distinct PRs plus 3 master commits in just 2 days:

This PR (#92866) modifies only MergedColumnOnlyOutputStream.cpp and MutateTask.cpp — neither touches FileCache code paths. The test failure is a timing race on slow builds, not a regression here.

Root cause

The test was added in #104313 to cover #104307. It drives the bug condition through FileCache::freeSpaceRatioKeepingThreadFunc with keep_free_space_elements_ratio = 1.0, which causes cache->initialize() at src/Interpreters/FileCache/FileCache.cpp:448 to schedule the asynchronous background eviction task. That task races with the test's read_and_check and intermediate assertProbationary / assertProtected calls. On slow builds (notably Unit tests (amd_llvm_coverage)) the background thread evicts entries between the populate and assert steps, so getFileSegmentsNum() reports 3 instead of the expected 0 at line 1626.

Fix is already in flight

PR #104418 ("Fix flaky FileCacheTest.SLRUFreeSpaceKeepingProtectedOnly", open since 2026-05-08) replaces the racy integration test with a focused unit-level test that calls SLRUFileCachePriority::collectEvictionInfo directly with three protected-only entries via addForRestore(... QueueEntryType::SLRU_Protected ...). That reproduces the exact chassert precondition (probationary empty, protected populated, is_total_space_cleanup=true) without involving FileCache's asynchronous machinery, making it deterministic on every build flavor.

@kssenii was already cc'd as the filesystem-cache owner. PR is MERGEABLE and currently green except for an unrelated Fast test flake on 02932_refreshable_materialized_views_2 (separate chronic timeout, 11 failures / 10 PRs in 30d, not introduced by #104418).

TL;DR for #92866

Re-running Unit tests (amd_llvm_coverage) here will pass intermittently. The test will become deterministic once #104418 merges. No action needed from @amosbird.

CI report for the failure on this PR:
https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=92866&sha=b82648604f3e517985d455be11e7939bfe265704&name_0=PR&name_1=Unit%20tests%20%28amd_llvm_coverage%29

(session: cron:clickhouse-ci-task-worker:20260510-004500)

CI status note for the two non-success checks on commit fccb42d:

• AST fuzzer (amd_debug, targeted, old_compatibility) — Inconsistent AST formatting (STID: 1941-1bfa). This is an existing fuzzer-found bug unrelated to this PR (the failing query is ALTER TABLE ... ADD PROJECTION ... GROUP BY ... AS alias26) where the alias placement inside a function call mis-prints). It is being fixed in Fix inconsistent AST formatting for aliased IN inside a function call (STID 1941-1bfa) #105091.
• Code Review — the copilot subprocess exited non-OK (Copilot review failed after 3 attempts); this is a transient infra failure and is what currently makes "Mergeable Check" fail.

Merged master into the branch to re-run CI. The two failures look unrelated to this PR:

1. Code Review — Copilot review subprocess infra failure (ERROR: Copilot review failed after 3 attempts: copilot subprocess exited with non-OK status).
2. AST fuzzer (amd_debug, targeted, old_compatibility) — Logical error: 'Inconsistent AST formatting: the query: ...' — a roundtrip formatting issue for an ADD PROJECTION expression containing <=> (isNotDistinctFrom) with an AS alias. The same error reproduces on master and several unrelated PRs (queried from play.clickhouse.com):

pull_request_number  date
0                    2026-05-12   ← master
92866                2026-05-12
103234               2026-05-11
90043                2026-05-10
92340                2026-04-27

The PR itself only touches projection-checksum bookkeeping in vertical merge / mutation, with no parser/formatter changes — it cannot affect AST roundtrip behavior.

Merged the latest master (the branch was ~1400 commits behind). The merge was clean — only MutateTask.cpp auto-merged, and the PR's two changes are intact. I re-verified the correctness invariant on current master: the vertical-merge path still adds the projection checksums via MergedBlockOutputStream::finalizePartAsync (MergedBlockOutputStream.cpp:222), so removing the duplicate addition in MergedColumnOnlyOutputStream::fillChecksums remains correct, and the mutation path is covered by the new loop in MutateSomePartColumnsTask::finalize. Both changed files compile cleanly against current master. CI was green before the merge; all review threads are resolved.

LLVM Coverage Report

Changed lines: Changed C/C++ lines covered by tests: 14/16 (87.50%) | Lost baseline coverage (was covered on master, now uncovered in this PR): 1 line(s) · Uncovered code

Full report · Diff report

The only red check was `CH Inc sync` (private sync PR #44919), failing on a single integration test: `test_shared_merge_tree_backups/test.py::test_restore_multiple_tables_on_parallel_nodes` (Data does not matched after BACKUP/RESTORE).

This failure is unrelated to this PR and flaky:

• The test tables are `SharedMergeTree() ORDER BY tuple()` with no projections, so the changed code (MergedColumnOnlyOutputStream::fillChecksums and MutateTask, both iterating getProjectionParts()) is a no-op for them. The test exercises BACKUP/RESTORE, not mutations or vertical merges.
• The test passed on the in-job retry on the same binary (PASSED on gw0); a checksum regression would fail deterministically. Data does not matched after BACKUP/RESTORE on parallel-node restore is a known backup/restore replication race.

I re-triggered the failed private CI jobs (run 27053851237); the check should go green. The branch is approved, all review threads are resolved, and it merges cleanly with master.