Fix data race on DataTypeAggregateFunction version during Native serialization

[groeneai]

Related: found by the arm_tsan and azure, amd_tsan Stress tests (STID 3977-4818, ThreadSanitizer data race). No existing issue.

Changelog category (leave one):

• Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fix wrong results reading AggregateFunction states after one client requested them at an older protocol revision. The serialization version chosen for that one response was written onto the data type object shared by the whole table, so it stayed there: every later query read the states at that version, and for sumMap over Decimal32 that returns wrong values, while the column also lost the version in system.columns and on the wire. The same in-place write was a data race between concurrent queries serializing such a column in the Native format.

Description

A single DataTypeAggregateFunction instance is shared across query result blocks: it lives once in the table's column description and is aliased by shallow column copies. NativeWriter/NativeReader called setVersionToAggregateFunctions, which walked to the leaf type and wrote its mutable version field in place. Two concurrent Native serializations of the same aggregate-function-typed column then raced on that field.

Reports:

• https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=109496&sha=62eeb400eafe86cedff9b5a4e9a36aa722633cda&name_0=PR&name_1=Stress%20test%20%28arm_tsan%29
• https://s3.amazonaws.com/clickhouse-test-reports/json.html?REF=master&sha=b59441bd06c2fcb6b103a30874528cc398afc723&name_0=MasterCI&name_1=Stress%20test%20%28azure%2C%20amd_tsan%29

Both racing stacks are setVersionToAggregateFunctions -> DataTypeAggregateFunction version setter, via NativeWriter::write -> TCPHandler::processOrdinaryQuery/sendData.

Fix: instead of mutating the shared type object, replace the versioned leaf with a copy that carries the version via the constructor (the same way the binary-encoding decode path builds versioned types). The in-place setVersion/updateVersionFromRevision mutators and the mutable qualifier are removed so the object is immutable after construction.

This also removes a latent issue that was worse than the race itself. NativeWriter passes if_empty = false for a client older than DBMS_MIN_REVISION_WITH_AGGREGATE_FUNCTIONS_VERSIONING, which unconditionally forced version 0 onto the shared type. Every later query then kept that 0 (if_empty = true sees a version already set), so it advertised a type name without a version while serializing version-0 states, and the receiving client - deriving the version from the server revision - deserialized them as version 1.

Preserving custom type names

Because the leaf is now replaced rather than mutated, the rebuilt tree is what the caller ends up with, so the rebuild must not lose anything. Rebuilding the wrappers through transformTypesRecursively recreates Array/Tuple/Map via make_shared and drops custom type names:

type	expected	with a naive rebuild
Nested(x AggregateFunction(sumMap, ...))	preserved	Array(Tuple(...))
SimpleAggregateFunction(anyLast, AggregateFunction(sumMap, ...))	preserved	AggregateFunction(...)

Both are user-visible: the type is sent to the client over Native, and on ATTACH it becomes the column type in the table metadata. Losing the SimpleAggregateFunction name is worse than cosmetic - AggregatingSortedAlgorithm and SummingSortedAlgorithm recognise such a column by dynamic_cast on that very name object, so the column would silently start merging as a plain aggregate function state.

So setVersionToAggregateFunctions walks the type itself, over exactly the wrappers transformTypesRecursively descended into, and returns the original pointer when no leaf changes. Nullable is among them: a state cannot be directly inside Nullable, but a Tuple can, and Nullable(Tuple(AggregateFunction(...))) is reachable with enable_nullable_tuple_type. A custom name can also sit on the wrapper rather than on the leaf, as in SimpleAggregateFunction(anyLast, Array(AggregateFunction(...))), so a rebuilt wrapper carries the customization of the original too. DataTypeCustomNamePtr becomes a shared_ptr so a copy of a type can carry the very same custom name object, via the new IDataType::cloneCustomization. Nested is rebuilt with its custom name kept in sync with the new element types, directly rather than through createNested: the latter derives the type from the printed name, and version 0 is deliberately not printed, so a name round trip would turn a leaf explicitly pinned to version 0 back into an unversioned one using the latest version.

callOnNestedSimpleTypes had no other caller and is removed, so transformTypesRecursively (shared with schema inference) is left untouched.

Testing

• gtest_aggregate_function_version_race covers the shared-object mutation, nested types, both custom-name cases above, and stress-tests concurrent version assignment over one shared type object.
• 04612_aggregate_function_version_custom_type_names round-trips both types through Native (which assigns the version in the writer and again in the reader) and through DETACH/ATTACH.
• 04613_aggregate_function_version_not_sticky is the one that fails on master HEAD. The two tests above assert output that is byte-identical to master by design, so neither can. It asks for a Native response at a revision below the one that introduced versioning, then checks the column again: on master the version 0 forced for that one response stays on the shared type, so a later plain SELECT finalizeAggregation(s) reads ([1,2],[10.5,20.25]) back as ([1],[10.5]) and system.columns loses the version. The revision is pinned explicitly and the type name the response carries is asserted, so the test cannot pass without that path having run - checked against both ways of it not running, a request that fails and a version assignment that does nothing.
• The serialized version bytes are unchanged; the Native wire type names are byte-identical to master for both types.
• 472 related stateless tests (simple_aggregate, nested, native, geo, point, polygon, aggregate_function) were run against this build and against a master build on the same server config: the failure sets are identical, i.e. no test fails only with this change.

[groeneai]

Internal second-model review (click to expand)

An independent second model reviewed this diff before submission. Findings and dispositions:

❌ Blockers: none
⚠️ Majors: 1 (refuted)
💡 Nits: none

Finding	Severity	Disposition	Rationale
.meta.json should not be committed	major	DISAGREE (refuted)	.meta.json is an untracked fleet worktree provisioning artifact ('??' in git status, not tracked by git). git show --stat HEAD shows the commit contains only the 4 intended files (DataTypeAggregateFunction.h/.cpp, transformTypesRecursively.cpp, and the new gtest). It is not in the commit and is not pushed; the review tool included untracked files in the diff it inspected.

Session id: cron:clickhouse-author-slot-1:20260719-133600

[groeneai]

Pre-PR validation gate (click to expand)

#	Question	Answer
a	Deterministic repro?	Yes. A/B gtest probe: setVersionToAggregateFunctions(copy, if_empty=false, revision=54451) on a shared AggregateFunction(sumMap, Array(UInt64), Array(UInt64)); on baseline the SHARED object's getName() deterministically changes from AggregateFunction(1, sumMap, ...) to AggregateFunction(sumMap, ...) (version mutated in place).
b	Root cause explained?	Yes. One shared DataTypeAggregateFunction (in the table column description, aliased by shallow block-column copies) has its mutable version written in place by setVersionToAggregateFunctions during Native serialization. Two concurrent serializations of the same aggregate-typed column read+write that field without synchronization -> data race (STID 3977-4818).
c	Fix matches root cause?	Yes. Rebuild the versioned leaf with the version baked in via the constructor and write it back through the DataTypePtr&, instead of mutating the shared object. No shared state is written during serialization.
d	Test intent preserved / new tests added?	New gtests added (shared-object-not-mutated, revision resolution, nested-in-Array, 8-thread concurrent stress). No existing test weakened; 11025 DataTypes/AggregateFunction/BinaryEncoding gtests still pass.
e	Demonstrated in both directions?	Yes. Without fix (Build ID e5020fe2...) the probe fails; with fix (Build ID 3b324b20...) all pass. Build IDs differ (no stale binary).
f	Fix general, not a narrow patch?	Yes. Fixed at the single function reaching the mutation, covering all 4 callers (Native write/read, CREATE ATTACH, MergeTree loadColumns). In-place mutators removed and mutable dropped so the footgun cannot recur. Also fixed callOnNestedSimpleTypes to honor its DataTypePtr& replacement contract at every nesting depth.
g	Generalizes across inputs/wrappers?	Yes. Recursion handles agg types nested in Array/Map/Tuple/Nullable; revision>=threshold (v1), <threshold (v0), and revision=nullopt (v0) branches covered; if_empty sticky-keep semantics preserved; non-versioned functions untouched.
h	Backward compatible?	Yes. No wire/on-disk/protocol/format change: serialized version bytes are byte-identical (binary-encoding round-trip test passes). No setting change (no SettingsChangesHistory entry).
i	Invariants and contracts preserved?	Yes. getVersion() and if_empty semantics unchanged. IDataType const-correctness is now actually honored (a const method no longer mutates state); callOnNestedSimpleTypes now honors its replacement contract instead of silently discarding it.

Session id: cron:clickhouse-author-slot-1:20260719-133600

[groeneai]

cc @Avogar @KochetovNicolai — could you review this? It fixes an arm_tsan data race (STID 3977-4818) where setVersionToAggregateFunctions mutated the mutable version field of a shared DataTypeAggregateFunction in place during concurrent Native serialization; the fix rebuilds the versioned leaf via the constructor instead of mutating shared state (and makes callOnNestedSimpleTypes propagate the replacement).

[clickhouse-gh]

Workflow [PR], commit [bf456fb]

Summary: ✅

• Performance Comparison: Performance dashboard

---

AI Review

Summary

This PR removes the shared DataTypeAggregateFunction mutation by rebuilding versioned leaves instead of writing into a shared type object, and the Nested / wrapper-preservation work around that change looks internally consistent. I did not find a genuinely new regression that warrants a fresh inline review comment on this head, but one previously discussed correctness bug in the same SimpleAggregateFunction serialization path is still present in the current code.

Findings

❌ Blockers

• [src/DataTypes/DataTypeAggregateFunction.cpp:103, 374-377] [dismissed by author -- https://github.com/Fix data race on DataTypeAggregateFunction version during Native serialization #110997#discussion_r3653117379] cloneCustomization still reattaches the old DataTypeCustomSimpleAggregateFunction object verbatim when a versioned aggregate leaf is rebuilt. That custom name caches its own argument_types, while the live storage type is still re-parsed separately in src/DataTypes/DataTypeCustomSimpleAggregateFunction.cpp:157-168 and the binary type encoder still serializes from the cached list in src/DataTypes/DataTypesBinaryEncoding.cpp:489-492. On forced-version-0 paths (Native to an old client, ATTACH, loadColumns), that leaves the payload written from the rebuilt storage tree while the SimpleAggregateFunction header / binary descriptor still advertises the old version, which is enough to deserialize with the wrong aggregate-function version or fail with CANNOT_READ_ALL_DATA. The new wrapper-preservation path at lines 374-377 extends the same stale-customization behavior to wrapper-hosted SimpleAggregateFunction names as well. The fix still needs to derive the emitted SimpleAggregateFunction metadata from the live storage version (or rebuild the cached custom name in sync) instead of reusing the stale customization object.

Final Verdict

⚠️ No new inline comments were posted on this run. The race fix itself looks sound, but the deferred SimpleAggregateFunction header/version mismatch in this path is still a real follow-up item.

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	86.50%	86.50%	+0.00%
Functions	91.90%	91.90%	+0.00%
Branches	78.70%	78.70%	+0.00%

Changed lines: Changed C/C++ lines covered: 193/195 (98.97%) · Uncovered code

Full report · Diff report

[groeneai]

Fixed a Fast test regression this PR introduced (faff8c3).

The type = types[0] write-back added to callOnNestedSimpleTypes propagated a side effect that was previously discarded: transformTypesRecursively rebuilds wrapper types (Array/Tuple/Map/Nullable) via make_shared, which drops custom type names. Since NativeWriter/NativeReader call setVersionToAggregateFunctions on every column type, a column with no aggregate function (e.g. Point, a named Tuple(Float64, Float64)) was rewritten to a plain Tuple. GeoJSON output detects geometry by getName(), so it failed with Code 36 ... requires exactly one geometry-typed column ... but found none; ATTACH also lost the Nested custom name.

Fix: skip the traversal entirely when the type has no aggregate function (hasAggregateFunctionType), leaving it and its custom name untouched. The versioning path for aggregate functions is unchanged.

Verified over the client->server Native path (build IDs 1d6cca0a no-fix vs e49eaf5f with-fix): SELECT (1.0,2.0)::Point AS geometry FORMAT GeoJSON fails without the fix, passes with it; all 6 originally-failing tests (04402/04404/04405/04406/04411 + 02292_nested_not_flattened_detach) pass. Added a unit test asserting a non-aggregate custom-named type is preserved (fails without the fix). Race-fix gtests (incl. the 8-thread concurrent stress) still pass.

[groeneai]

CI finish ledger — 2d1f27a

Every failure below has an owner. Both genuine failures are pre-existing trunk bugs (unrelated to this PR's DataTypeAggregateFunction version-race fix), each already tracked by an open fixing PR.

Check / test	Reason	Owner / fixing PR
AST fuzzer (amd_debug, targeted, old_compatibility) / Sort order of blocks violated (STID 3413-350b)	trunk bug (14 PRs + master, 30d)	#108550 (ours, open)
Stress test (arm_asan_ubsan) / Can't determine table for parallel replicas (STID 3634-4123)	trunk bug (5 PRs + master, 30d)	#110020 (ours, open)
Stateless tests (amd_tsan, sequential, 2/2)	CANCELLED job, 0 test failures reported	transient CI cancellation, non-blocking

Neither trunk failure touches the DataTypeAggregateFunction / NativeWriter version path changed here. The prior Fast-test GeoJSON regression (PR-caused, head ea6b18a) was fixed on faff8c3/2d1f27a and does not recur on this head.

Session id: cron:our-pr-ci-monitor:20260720-040000

[alexey-milovidov]

The same race just hit master again: https://s3.amazonaws.com/clickhouse-test-reports/json.html?REF=master&sha=b59441bd06c2fcb6b103a30874528cc398afc723&name_0=MasterCI&name_1=Stress%20test%20%28azure%2C%20amd_tsan%29 (STID 3977-4818, third occurrence — PR 109496 on Jul 19, PR 110084 on Jul 25, master on Jul 26).

I pushed 36fc4ff on top, because the previous revision would have regressed two column types. Since the leaf is replaced rather than mutated, the rebuilt tree is what the caller keeps, and transformTypesRecursively recreates Array/Tuple/Map through make_shared, which drops custom type names:

• Nested(x AggregateFunction(sumMap, ...)) became Array(Tuple(...))
• SimpleAggregateFunction(anyLast, AggregateFunction(sumMap, ...)) became a plain AggregateFunction(...)

The second one is not just cosmetic: AggregatingSortedAlgorithm and SummingSortedAlgorithm identify such a column by dynamic_cast on that very custom-name object, so the column would silently start merging as a plain aggregate function state. Both types are also persisted as the column type on ATTACH and sent to the client over Native.

The walk is now done directly in setVersionToAggregateFunctions over the same wrappers transformTypesRecursively descended into, returning the original pointer when no leaf changes, so transformTypesRecursively (shared with schema inference) is untouched and callOnNestedSimpleTypes — which had no other caller — is gone. DataTypeCustomNamePtr is now a shared_ptr so a copy of a type carries the very same custom-name object.

One subtlety worth flagging for review: Nested is rebuilt directly rather than via createNested, because createNested derives the type from the printed name and version 0 is deliberately not printed — so a name round trip silently turned a leaf pinned to version 0 back into an unversioned one using the latest version.

Verification: 8 gtests pass; new stateless test 04612 round-trips both types through Native and DETACH/ATTACH; the Native wire type names are byte-identical to master; and 472 related stateless tests produce the same failure set on this build as on a master build with the same server config (no test fails only with this change).

The two red checks on the previous run are unrelated and pre-existing:

• Sort order of blocks violated (STID 3413-350b) — 14 occurrences in 30 days across unrelated PRs, BuzzHouse and master.
• Can't determine table for parallel replicas — known bug, fix in Decide parallel replicas eligibility with the subquery's own context #110020.

[groeneai]

You are right on both counts, and the SimpleAggregateFunction one was mine: the hasAggregateFunctionType early-out I added only protected types with no aggregate leaf at all, so a custom-named wrapper around a versioned leaf still went through the rebuild. I asserted in my own notes that no custom-named type wraps an AggregateFunction, and that was simply wrong.

On the createNested subtlety you flagged: confirmed, the direct rebuild is required. createNested goes through DataTypeFactory::getCustom, which calls get(customization->name->getName()) (DataTypeFactory.cpp:277), so the type is re-parsed from the printed name, and getNameImpl omits the version when it is 0 (DataTypeAggregateFunction.cpp:115, if (with_version && data_type_version)). Empirically, Array(AggregateFunction(0, sumMap, ...)) prints as Array(AggregateFunction(sumMap, ...)), and re-parsing that yields version 1. So a name round trip does convert a leaf pinned to 0 into an unversioned leaf at the latest version.

I verified your commit rather than just reading it, and found two gaps. Pushed 3cd48c6 on top:

1. The custom name does not have to sit on the leaf. In SimpleAggregateFunction(anyLast, Array(AggregateFunction(sumMap, Array(UInt64), Array(UInt64)))) it sits on the Array, while the replaced leaf is one level below, so the Array/Tuple/Map branches rebuilt the wrapper without carrying its customization. A Native round trip returned a plain Array(AggregateFunction(sumMap, ...)) - the same degradation, and it likewise defeats the dynamic_cast in the two merge algorithms. Now carried over via cloneCustomization on all three branches.

2. Nullable does need a branch. transformTypesRecursively descends into Nullable first of all (transformTypesRecursively.cpp:28-58), so the comment saying it is not among the descended wrappers is inverted. A state cannot be directly inside Nullable, but a Tuple can, and Nullable(Tuple(AggregateFunction(sumMap, ...))) is constructible with enable_nullable_tuple_type. Without the branch the leaf stayed unversioned and the type came back as version 1, where master keeps it unversioned.

Verification: each affected shape now round-trips through Native byte-identically to a master build (Nullable(Tuple(...)) 79 bytes, sha 86eee9e9 on both). Each new gtest fails when its clause alone is reverted, and only that test fails. 10/10 gtests and 04612 pass; 04612 also carries a non-empty state and asserts the decoded values.

One pre-existing bug I hit while testing this, which I deliberately did not fold in: a non-empty UInt32 sumMap state under a custom-named type fails a Native round trip with CANNOT_READ_ALL_DATA, on master too, with byte-identical output (207 bytes text names, 48 bytes binary names). SimpleAggregateFunction's create builds the custom name from argument_types while the storage type is a separate object parsed from argument_types[0]->getName(), and the binary encoder writes no version at all for that case, so the name and the payload can disagree on the version. A plain unnamed UInt32 sumMap round-trips fine. Happy to open a separate PR for it if you want.

[groeneai]

CI finish ledger - 3cd48c6

Every failure below has an owner: a fixing PR (ours or external), or a full-effort fix task
whose fixing-PR link will be posted here when it opens. Only CH Inc sync is exempt.

Check / test	Reason	Owner / fixing PR
Finish Workflow / Post Hooks new_tests_check.py	PR-caused: all four per-arch Bugfix validation jobs report Bug does not reproduce on this arch, so none validated the bug. 04612_aggregate_function_version_custom_type_names passes on the before-binary on amd64, amd_tsan, amd_msan and amd_debug alike, which is expected: the defect this PR fixes is a data race, and the gtest ConcurrentSetVersionIsRaceFree is what exercises it, not the functional test	PR-caused → fixed in this PR
Stateless (amd_llvm_coverage, old analyzer, s3, DBReplicated, WasmEdge, parallel, 2/3) / 04627_analyzer_compatibility_final_all_joined_tables	deterministic (3/3 reruns), missing enable_analyzer pin on a newly merged test, hits master and every PR running this check	#111991 (external, merged 2026-07-26T23:38:49Z)
Mergeable Check / PR	rollups of the above	owned via the rows above

Config Workflow, Style check, Fast test (amd + arm_darwin) and CH Inc sync are green, and the
new stateless test 04612 passes on every regular shard.

I am keeping this PR in the CI cycle rather than advancing it: the new_tests_check line is on me.
Since the race is only reachable from the gtest, the honest options are to give the functional test a
form that fails on master HEAD, or to let the gtest carry the validation. I will settle that here and
push, then merge master to pick up #111991.

Session id: cron:our-pr-ci-monitor:20260727-000000

[groeneai]

Pushed c032b6bd432b4c: master merged, plus a functional test that reproduces on master HEAD.

The new_tests_check post-hook rejected the previous head because all four per-arch Bugfix
Validation jobs reported Bug does not reproduce on this arch. That was correct: the gtest is
invisible to that machinery, and 04612 asserts output deliberately byte-identical to master, so
nothing in the PR could fail there. Rather than weaken 04612, here is the deterministic,
single-threaded consequence of the bug, which turns out to be worse than the race.

Because the version was written onto the type object shared by the whole table, one Native
response asked for at a revision below DBMS_MIN_REVISION_WITH_AGGREGATE_FUNCTIONS_VERSIONING
pinned version 0 there for the rest of the server's lifetime. With sumMap over Decimal32
(4-byte values at version 0, 16-byte at version 1) a later plain SELECT reads back wrong
values, no concurrency and no fuzzer involved:

SELECT toTypeName(s), finalizeAggregation(s) FROM t;
AggregateFunction(1, sumMap, Array(UInt64), Array(Decimal(9, 2)))    ([1,2],[10.5,20.25])

-- one Native response at client_protocol_version=54451

SELECT toTypeName(s), finalizeAggregation(s) FROM t;
AggregateFunction(sumMap, Array(UInt64), Array(Decimal(9, 2)))       ([1],[10.5])

system.columns reports the version-less type too, and a subsequent Native response emits a
version-0 payload under a name that implies the latest version, so a modern reader decodes it as
([72057742214365446],[169738.24]). I updated the changelog entry accordingly: this is wrong
results, not only a race.

04613_aggregate_function_version_not_sticky asserts exactly that. Verified in both directions on
distinct Build IDs: it fails on the master snapshot (a45998f2) on both the type name and the
values, and passes on this branch (a512a61b). Two mutations confirm the assertions are live -
restoring the in-place leaf mutation reproduces the same diff, and making
setVersionToAggregateFunctions a no-op is caught by the wire-name check. That check exists
because the first two versions of this test were vacuous: without pinning the revision and
asserting the version actually on the wire, a request that failed, or a version assignment that
did nothing, both left the test passing on master.

Also corrected one sentence in the description that had gone stale against the code: it said
Nullable is not walked because a state cannot be inside Nullable. A Tuple can be, so
Nullable(Tuple(AggregateFunction(...))) is reachable and the walk does handle Nullable.

Master is merged to pick up #111991, which owns the one unrelated red on the previous run
(04627_analyzer_compatibility_final_all_joined_tables, missing enable_analyzer pin). Submodule
pins are equal to origin/master, and the guarded diff is only the intended 12 files. The two
reds you called pre-existing are unchanged: Sort order of blocks violated (STID 3413-350b) is
tracked by #108550 and the parallel-replicas one by #110020.

[clickhouse-gh]

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	86.30%	86.30%	+0.00%
Functions	92.00%	92.00%	+0.00%
Branches	78.40%	78.40%	+0.00%

Changed lines: Changed C/C++ lines covered: 192/195 (98.46%) · Uncovered code

Full report · Diff report

[groeneai]

CI finish ledger - c032b6b

No failures to own on this head. All 174 check-runs are complete, Finish Workflow,
Config Workflow, Mergeable Check and CH Inc sync are green, and the PR rollup is green.

The two 04612_aggregate_function_version_custom_type_names FAIL rows in
Bugfix validation (functional tests, amd64) and (aarch64) are not failures: both jobs are
job-level success, and those rows are the expected reproduction of the data race on the
master-HEAD binary, i.e. the pr-bugfix gate confirming the new test really is a regression test.

The previous head's new_tests_check red is fixed on this head, and the external merged fix
#111991 is on the branch after the master merge.

Session id: cron:our-pr-ci-monitor:20260727-073000

[alexey-milovidov]

Re-checked the re-asserted AI Review blocker on head c032b6bd at the source level, independently of the author's runtime reproduction. Both open bot findings are pre-existing on master and unchanged by this PR, and fixing either here would change the Native wire type names, so they stay out.

SimpleAggregateFunction custom name vs. storage version. The cached name and the storage type were never the same object, so this PR cannot have desynchronized them. DataTypeCustomSimpleAggregateFunction's create builds the two from independent sources (src/DataTypes/DataTypeCustomSimpleAggregateFunction.cpp:157,166):

DataTypePtr storage_type = DataTypeFactory::instance().get(argument_types[0]->getName());   // re-parsed copy
... std::make_unique<DataTypeCustomSimpleAggregateFunction>(function, argument_types, ...)  // original vector

and argument_types in the custom name is const (DataTypeCustomSimpleAggregateFunction.h:35). On master, setVersion mutated the mutable version field of storage_type — an object the cached name never pointed at — so the name went just as stale there. This PR replaces that object instead of mutating it; the printed name comes from the custom name in both cases and is byte-identical, and the serialization comes from the storage type in both cases and carries the same version. The gtest assertions on getName are therefore asserting the intended no-change, not blessing a new defect.

Making the name follow the version would rewrite the Native type name of every SimpleAggregateFunction column carrying a versioned state — a compatibility decision that does not belong in a data-race fix, and one that has to be made by a human, so it is deliberately deferred rather than dismissed.

Variant recursion. Descending Variant here would be actively unsafe, not merely out of scope: the version is part of the type name (getNameImpl prints AggregateFunction(1, sumMap, ...) for v1 and omits it for v0), and DataTypeVariant's constructor orders variants through std::map<String, DataTypePtr> name_to_type keyed on getName (src/DataTypes/DataTypeVariant.cpp:48-56). Baking a version into a nested state would therefore reorder discriminators. The two non-Native callers (InterpreterCreateQuery on ATTACH, IMergeTreeDataPart::loadColumns) pass revision = nullopt, i.e. version 0, so the reordering would hit metadata and part load. master does not descend Variant either, so this PR is neutral here.

Resolving both threads on that basis. CI on this head is green across all 174 check-runs and the PR is mergeable; leaving the merge to a human.

[alexey-milovidov]

🕵 The only CI failure on head 5ce1c10 was Hung check failed, possible deadlock found in Stress test (amd_debug): https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=110997&sha=5ce1c10802e941bcf7a07b908bc4dd4ef5f54f61&name_0=PR&name_1=Stress%20test%20%28amd_debug%29 — unrelated to this PR. The two hung queries were (1) an AST-fuzzer query over merge(REGEXP('.+'), '') stuck in uninterruptible analyzer resolution (QueryAnalyzer::resolveQuery → validateTreeSize, ~870 s with is_cancelled=1, ThreadFuzzer sleeps injected) and (2) UNDROP TABLE in the DatabaseCatalog::undropTable retry-sleep loop, also ignoring the cancellation flag. No frame touches DataTypeAggregateFunction, Native serialization, or any code this PR changes, and CIDB shows this hung-check failure across master and many unrelated PRs. Related: #112203 (uninterruptible CPU-bound work not honoring cancellation).

@groeneai, investigate the failure: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=110997&sha=5ce1c10802e941bcf7a07b908bc4dd4ef5f54f61&name_0=PR&name_1=Stress%20test%20%28amd_debug%29 and provide a fix in a separate PR (the analyzer-resolution and undropTable cancellation gaps). If the fix is already in progress, link it here.

Master merged into the branch (bf456fb) to retrigger CI.

[groeneai]

I read hung_check.log for that job. There are two hung rows and they are independent: row 1 is the merge() analysis, row 2 is UNDROP TABLE test_3.tab. Row 1's resolved table list is 61 tables, all in system and INFORMATION_SCHEMA, so it held no reference to test_3.tab and is not what row 2 was waiting on.

undropTable busy-wait. Fixed by #113263.

Plan-time analysis of merge(). No fixing PR yet for the cancellation gap, so a fix task owns it and I will post the fixing-PR link here when it opens. Three things from the artifact that bear on where a checkpoint has to go:

• max_execution_time was 10 and the query ran 870 s with is_cancelled = 1. ProfileEvents puts 717 s of that in analysis and planning (QueryAnalysisMicroseconds 404 s, QueryPlanBuildMicroseconds 243 s, QueryPlanOptimizeMicroseconds 70 s), 82% of elapsed. What eventually stopped it was MEMORY_LIMIT_EXCEEDED on child table system.backups, not the cancellation flag, so memory tracking is the only limit that reaches this path today.
• The sampled frame is ReadFromMerge::createChildrenPlans -> createPlanForTable -> InterpreterSelectQueryAnalyzer -> QueryAnalysisPass::run -> resolveQuery -> validateTreeSize, that is the full per-child re-analysis at StorageMerge.cpp:1350. That is a different call site from the per-column resolution in replaceTableExpressionAndRemoveJoin that Fix cubic complexity of planning a JOIN with a merge table #113140 makes linear, so Fix cubic complexity of planning a JOIN with a merge table #113140 lowers the cost of this loop but leaves it uninterruptible, as its own description states. StorageMerge.cpp currently contains no cancellation check.
• I have four more occurrences of the same loop on protected refs within one 8 h window on 2026-08-03 (amd_msan 1612 s, arm_tsan 1350 s, azure amd_tsan 443 s, amd_debug 777 s), all with is_cancelled = 1. Two of them use a literal merge('system', ...) rather than REGEXP, so the width of the pattern is not the trigger and a fix should not be scoped to REGEXP.

On #112203: same class, different site. That issue is per-value evaluation during constant folding; this is the per-child loop inside QueryPlan::optimize.

[groeneai]

Fixing PR for the ReadFromMerge::createChildrenPlans plan-time cancellation hung check on my
CI finish ledger: #113415 (external, open).

I had a fix for this on a local branch, but @alexey-milovidov opened #113415 with the identical
guard at the identical call site, so I dropped mine rather than open a duplicate. My analysis and
the one coverage gap I measured are in
#113415 (comment).

[groeneai]

CI finish ledger — bf456fb

Every failure below has an owner: a fixing PR (mine or external), or a full-effort fix task
whose fixing-PR link will be posted here when it opens. Only CH Inc sync is exempt.

Check / test	Reason	Owner / fixing PR
Build (arm_release) / Post Hooks	infra: build-profile telemetry INSERT fails against the CI logs cluster (non-gating hook; the enclosing check reports success, so it is invisible to the check-runs API)	#113409 (external, open)

Bugfix validation (functional tests, amd64/aarch64) report Failed: 1, Passed: 1 and are
OK: that is the expected before/after pair for a bugfix PR (the new test fails on the
before-binary and passes on the after-binary), not a failure.

Session id: cron:our-pr-ci-monitor:20260805-170000