Add `GradualResizeProcessor` to limit effective parallelism for GROUP BY on small data volumes by alexey-milovidov · Pull Request #99495 · ClickHouse/ClickHouse

alexey-milovidov · 2026-03-14T07:35:33Z

When ClickHouse processes GROUP BY, it often overestimates the number of threads needed. With max_threads = 64 but only a few thousand rows, all 64 AggregatingTransform instances get data, produce 64 partial hash tables, and the merge phase has to combine all of them — most nearly empty. This wastes time on merging overhead, which is especially noticeable for heavy aggregate states such as uniq, uniqExact, groupArray, etc.

The new GradualResizeProcessor starts by pushing data to a single output port (or one port per split group when min_outstreams_per_resize_after_split applies), and activates all aggregation streams at once as soon as the configured row or byte threshold is crossed. For small datasets, only one aggregating thread receives data (or one per split group); for large datasets, all threads are used as before.

New settings:

min_rows_per_stream_for_gradual_resize (default: 0)
min_bytes_per_stream_for_gradual_resize (default: 0)

Both settings default to 0, so the optimization is opt-in. Set either setting to a non-zero threshold to replace the pre-aggregation StrictResize with GradualResize for keyed GROUP BY queries.

Changelog category (leave one):

Performance Improvement

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

Add opt-in settings min_rows_per_stream_for_gradual_resize and min_bytes_per_stream_for_gradual_resize that can improve GROUP BY performance on small data volumes.

Workflow [PR]
Sync PR [sync-upstream/pr/99495]

When ClickHouse processes GROUP BY, it often overestimates the number of threads needed. With `max_threads = 64` but only a few thousand rows, all 64 `AggregatingTransform` instances get data, produce 64 partial hash tables, and the merge phase has to combine all of them — most nearly empty. This wastes time on merging overhead. The new `GradualResizeProcessor` starts by pushing data to only 1 output port, and gradually activates more ports as data volume grows. For small datasets, only 1-2 aggregating threads receive data; for large datasets, all threads are used as before. New settings (both default to 0 = disabled): - `min_rows_per_stream_for_gradual_resize` - `min_bytes_per_stream_for_gradual_resize` When either threshold is non-zero, the pre-aggregation `StrictResize` is replaced with `GradualResize` in the pipeline. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

clickhouse-gh · 2026-03-14T07:36:12Z

Workflow [PR], commit [0503bb8]

Summary: ✅

Performance Comparison: Performance dashboard

AI Review

Summary

This PR adds an opt-in GradualResizeProcessor for keyed GROUP BY pre-aggregation, wires the new thresholds through planning and pipeline building, preserves the relevant step state across cloning and query-plan serialization, and documents/tests the main no-op carriers (GROUPING SETS, constant keys, aggregate projections, lazy FINAL, and shipped plans over evenly distributed reads). On the current head I did not find a remaining correctness, compatibility, or rollout issue that merits a new inline finding.

Final Verdict

✅ No new blockers or majors. The remaining question is workload value and tuning rather than correctness: the feature is disabled by default and the current code/docs make that opt-in contract explicit.

LLVM Coverage Report

Measured on commit 0503bb8.

Metric	Baseline	Current	Δ
Lines	89.00%	89.00%	+0.00%
Functions	91.80%	91.80%	+0.00%
Branches	81.40%	81.40%	+0.00%

Changed lines: Changed C/C++ lines covered: 397/419 (94.75%) · Uncovered code

Full report · Diff report

ClickBench Q36-Q42 benchmarks show 1.11x geometric mean speedup with `min_rows_per_stream_for_gradual_resize = 1000`: - Q40 (426K-group filtered aggregate): 40ms → 29ms (1.38x) - Q38 (42K groups): 18ms → 14ms (1.29x) - Q37 (299K groups): 22ms → 19ms (1.16x) - Q36 (full-table 9.1M groups): 36ms → 38ms (5% regression, acceptable) - Q39, Q41, Q42 (very selective): neutral Higher thresholds (50K+ rows) cause regressions on large datasets by keeping parallelism too low for too long. 1000 rows/stream is the sweet spot: conservative enough to ramp up quickly for heavy queries, but effective at reducing merge overhead for filtered aggregates. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Skip finished outputs when dequeuing from `active_waiting_outputs` and when promoting entries from `inactive_waiting_outputs`. Without these guards, a downstream cancellation could mark an output as `Finished` while it was still enqueued, leading to an invalid `pushData` call. Also disable `min_rows_per_stream_for_gradual_resize` in three existing tests that validate pipeline structure or thread-count assumptions unrelated to gradual resize. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Fix stateless test `04039_gradual_resize_processor`: set `enable_analyzer = 1` at session level so the test works in old-analyzer CI configurations - Fix `test_storage_mysql::test_many_connections`: disable `GradualResize` for this test because 25 UNION ALL branches with a MySQL connection pool of 16 causes pool exhaustion — `GradualResize` bottlenecks data through 1 output initially, so MySQL sources hold connections longer and waiting sources time out Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…x performance regressions The original `GradualResizeProcessor` replaced `StrictResizeProcessor` in the aggregation pipeline but used non-strict FIFO routing — any input could feed any output. This destroyed thread/cache locality and caused regressions on large datasets: - `group_array_sorted`: up to +146% slower on AMD, +39% on ARM - `sort_patterns #4`: +408% on ARM - `optimize_functions_to_subcolumns`: +15-50% on AMD Root cause: even after all outputs activated (which happens after the first 65K-row chunk with threshold=1000), data was routed arbitrarily instead of maintaining the 1:1 input-output binding that `StrictResizeProcessor` provides. Fix: rewrite `GradualResizeProcessor` to use strict routing (like `StrictResizeProcessor`). Inputs start disabled and are only enabled when bound to a specific active output. This preserves thread locality for large datasets while still providing the gradual activation benefit for small GROUP BY queries. Performance report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=99495&sha=edb0902ed32ccbbf461112a71a027e24b1cdfcd2&name_0=PR Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

nihalzp · 2026-03-17T20:01:03Z

In this PR, we try to gradually increase the active input and output ports as incoming data reaches our processor and crosses some threshold. The issue with this is that this can lead stalling in the upstream as well (as discovered by the performance tests).

This happens because since at the start most of the input ports (except the first one) has not set setNeeded, the upstream transform sees that it cannot push which then propagates and linearize the pipeline till we have reached enough rows to unlock more ports. This can be bad if the previous transform is a FilterTransform which filters out all of the rows, and our entire pipeline stays linear forever (to be more precision, parallelism collapses to the current number of active outputs.)

One idea could be that we keep all the input ports and gradually allow output ports. We try to push the input chunk to any of the available allowed output chunks; in this way this will not linearize the pipeline while keeping most of the benefit.

alexey-milovidov · 2026-03-18T15:18:13Z

One idea could be that we keep all the input ports and gradually allow output ports.

I actually meant it this way. Will try...

…ually activate outputs Address review feedback from #99495: - Keep all input ports active at all times to avoid upstream pipeline stalling. Previously, inputs were disabled by default and bound 1:1 to outputs, which caused upstream transforms (e.g. `FilterTransform`) to stall when most inputs were inactive. Now uses many-to-many routing (like `ResizeProcessor`) with all inputs always active, while only gradually activating output ports as data volume grows. - Fix potential integer overflow in `maybeActivateMoreOutputs` by using division-based comparison instead of multiplication (`total_rows / num_active >= threshold`). - Remove `.claude/learnings.md` artifact file that was accidentally included. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…ke `StrictResizeProcessor` The original implementation enabled all inputs unconditionally, used decoupled queues for routing, and called `getNumRows`/`bytes` on every chunk even after all outputs were activated. This caused measurable steady-state overhead vs `StrictResizeProcessor`, leading to performance regressions in CI perf tests. The new implementation adopts `StrictResizeProcessor`'s pattern: - Each input is paired 1:1 with an output via `waiting_output` - Inputs are disabled after delivering data (`pullData(set_not_needed=true)`) - Inputs are only re-enabled when an output needs data (demand-driven) - Per-chunk row/byte accounting is skipped once all outputs are active (`all_outputs_active` flag) Once all outputs are activated, the processor behaves identically to `StrictResizeProcessor` with zero additional overhead. CI perf report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=99495&sha=edb0902ed32ccbbf461112a71a027e24b1cdfcd2&name_0=PR Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…mplementation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…istory` Test `02995_new_settings_history` checks that newly added settings are in versions strictly greater than `26.4`. Both `min_rows_per_stream_for_gradual_resize` and `min_bytes_per_stream_for_gradual_resize` were previously placed in the `26.4` block, so the test reported them as missing. CI report: ClickHouse#99495 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Set `min_rows_per_stream_for_gradual_resize` default to `1000` so the gradual-activation behavior for `GROUP BY` aggregation streams is on out of the box. Users still pay the merge-overhead cost for small result sets only when they explicitly opt out by setting it to `0`. Per review thread: ClickHouse#99495 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Replace the one-output-at-a-time ramp in `maybeActivateMoreOutputs` with a single jump to all outputs once `total_rows_pushed >= min_rows_per_output` (or the bytes equivalent) is true. Why: the gradual ramp made early outputs receive disproportionately more chunks during the warm-up phase, leaving aggregator #0 with a much larger partial hash table than the rest. That imbalance persists for the whole run, and the downstream merge has to combine N uneven partial states. For heavy aggregate states the merge cost scales super-linearly with table size, so the skew hurts even on large queries that should clearly saturate all aggregating threads. Performance evidence on `f9aea621a2cb`: - `group_array_sorted` from `sorted_50m`/`sorted_100m`: +100%..+150% - `hash_table_sizes_stats` `numbers(5M) GROUP BY number`: +55% - `tpch` Q22: +22%/+37% With "jump to all", small queries still see only 1 aggregator (under the configured threshold), and any query past the threshold immediately uses the full N — bounding the early skew to at most `min_rows_per_output` rows instead of `min_rows_per_output * N / 2`. Linked discussion: ClickHouse#99495 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

When `num_streams` is not divisible by the number of split groups, `addSplitResizeTransform` pads the last group with `NullSource` / `NullSink`. The padded `NullSink` output is finished immediately by `NullSink::prepare`, so `GradualResizeProcessor::prepare` sees it as `Finished` before any data flows and never adds it to `waiting_outputs` — even after `maybeActivateMoreOutputs` sets `all_outputs_active`. This test exercises that path with 14 upstream streams and `min_outstreams_per_resize_after_split = 4` (groups = 3, each 5x5; the third group has one padded input and one padded output) and checks that the query returns all 140000 rows, i.e. no data is dropped on the padded path. Addresses review thread on ClickHouse#99495.

Cover the branch in `GradualResizeProcessor::prepare` that promotes a waiting inactive output when an *active* output finishes before the row/byte threshold is crossed. The existing `04039_gradual_resize_processor.sql` only exercises finished *inactive* outputs (padded `NullSink` after `addSplitResizeTransform`); this test forces the deadlock-avoidance path by raising `min_rows_per_stream_for_gradual_resize` above the input size so the row threshold never fires, then using `max_rows_to_group_by` with `group_by_overflow_mode = 'break'` to make the active `AggregatingTransform` close its input port (= the `GradualResize` output port) after a single chunk. Addresses outstanding review thread on `04039_gradual_resize_processor.sql` (thread `PRRT_kwDOA5dJV85_IY73`) in ClickHouse#99495. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

`LimitTransform::makeChunkWithPreviousRow` asserts `row < chunk.getNumRows()` and is called from `preparePair` with `data.current_chunk.getNumRows() - 1` whenever `with_ties && rows_read == offset + limit`. This silently assumes the current chunk has at least one row. The read-in-order pipeline violates that assumption. With settings `optimize_read_in_order = 1`, `read_in_order_use_virtual_row = 1` and `read_in_order_use_virtual_row_per_block = 1`, `VirtualRowTransform` emits zero-row chunks carrying `MergeTreeReadInfo` as sort-key markers for `MergingSortedTransform`. In the single-stream case there is no merging, so `RemoveVirtualRowTransform` is inserted to clean up — but it only stripped the `ChunkInfo` and left the empty chunk on the pipeline. When such a chunk arrived at `LimitTransform` after the previous data chunk had pushed `rows_read` to exactly `offset + limit`, the call became `makeChunkWithPreviousRow(empty_chunk, 0 - 1)` and the `UInt64` underflow tripped the assertion (server abort in debug builds; surfaces as a `LOGICAL_ERROR` in release builds). The AST fuzzer hit this on master across multiple unrelated PRs (ClickHouse#99495, ClickHouse#104268, ClickHouse#101158) over the past 30 days. Minimal reproducer: CREATE TABLE t (id UInt64, m Map(String, String), INDEX idx_mk mapKeys(m) TYPE bloom_filter GRANULARITY 1) ENGINE = MergeTree ORDER BY id SETTINGS index_granularity = 1; INSERT INTO t VALUES (1, {'1':'1'}); INSERT INTO t VALUES (2, {'2':'2'}); INSERT INTO t VALUES (3, {'3':'3'}); SELECT id FROM t PREWHERE mapContains(m, toFixedString('1', 1)) ORDER BY id ASC LIMIT 1 WITH TIES SETTINGS optimize_read_in_order = 1, read_in_order_use_virtual_row = 1, read_in_order_use_virtual_row_per_block = 1; Two fixes: 1. `LimitTransform.cpp`: skip the `makeChunkWithPreviousRow` call when the current chunk has zero rows. There is no row to remember and the `previous_row_chunk` saved from the prior non-empty chunk is still the correct tie key for the boundary. 2. `RemoveVirtualRowTransform` (`SortingStep.cpp`): actually consume the virtual-row chunk — `chunk.clear()` it and enable `skip_empty_chunks` on the base `ISimpleTransform` so it is not re-emitted. This matches the existing comment ("we need to remove virtual row before output") and protects other downstream transforms from receiving these pointless markers. Regression test `04247_99495_limit_with_ties_virtual_row_empty_chunk` aborts master in debug builds and passes after this change. CI report: https://s3.amazonaws.com/clickhouse-test-reports/PRs/99495/b699481fd40feb67d515e47725de159f20dd1ff0/ast_fuzzer_amd_debug/ Related: ClickHouse#99495 (sighting PR)

…essor # Conflicts: # src/Core/ProtocolDefines.h # src/Core/SettingsChangesHistory.cpp

…rebased `AggregationPushdown` clones the planned `GROUP BY` step and calls `rebaseOntoInput` with the join keys it pushes the aggregation below, but `group_by_keys_semantically_constant` was decided by the planner for the *original* key set. Carrying it over kept claiming a single group for a step that now groups by the join keys, so the gradual pre-aggregation resize the user opted into with `min_rows_per_stream_for_gradual_resize` silently stopped applying on that carrier. `rebaseOntoInput` now drops the mark, exactly like the hash-table stats identity its caller resets for the same reason; the remaining header-based `ColumnConst` check still catches keys that are constant in the new input. New `AggregatingStep.RebaseOntoInputClearsSemanticallyConstantKeys` unit test with both halves of the pair: the mark alone keeps the strict resize, and the rebased step takes the gradual one. The functional surface is not reachable without a stateless worker cluster - the pushed partial aggregation runs on a worker, so it shows up in neither `EXPLAIN PIPELINE` nor `system.processors_profile_log` of the initiator. Addresses the review thread on `AggregatingStep.cpp:754`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

alexey-milovidov · 2026-09-12T03:20:01Z

🕵 Round 307 (head 0bf8056a0fdb, previous a61fdc38d865): master merged with the expected plan-serialization version collision resolved, and all three open AI Review findings addressed.

Master merge (fccd727a2a26) — the branch was 2237 commits behind and CONFLICTING, single merge base, two conflicts:

src/Core/ProtocolDefines.h — master took both 15 (LimitRange step) and 16 (JoinStepLogical estimate-derived decisions) for DBMS_QUERY_PLAN_SERIALIZATION_VERSION, the collision anticipated in the previous rounds. The second AggregatingStep flags byte of this branch is renumbered to 17, with DBMS_MIN_QUERY_PLAN_SERIALIZATION_VERSION_WITH_SEMANTICALLY_CONSTANT_GROUP_BY_KEYS = 17; master's two version comments are kept verbatim and this branch's is appended after them. No behavioural change — every gate is symbolic.
src/Core/SettingsChangesHistory.cpp — union of both entries; the two *_for_gradual_resize entries are still inside the in-progress 26.9 block (autogenerated_versions.txt is still 26.9).

AI Review Major — stale constant-key mark on the Cascades pushdown carrier (0bf8056a0fdb). Adopted with the suggested fix: AggregatingStep::rebaseOntoInput clears group_by_keys_semantically_constant, because the mark was decided by the planner for the key set that the rebase replaces — the same reason the caller resets the hash-table stats identity one line earlier. Without it, a pushed partial aggregation kept claiming a single group after being rebased onto the join keys, so the opt-in gradual resize silently stopped applying there. Pinned by a unit test rather than a stateless one: the pushed partial aggregation runs on a worker, so the initiator sees only ReadFromDistributedPlanSource in EXPLAIN PIPELINE and nothing of that pipeline in its own system.processors_profile_log. AggregatingStep.RebaseOntoInputClearsSemanticallyConstantKeys asserts both halves: marked → strict Resize, marked + rebased → GradualResize. Thread.

AI Review Tests — 04262 had no positive control. Both halves now take an EXPLAIN PIPELINE control with the settings of the query that follows (GradualResize, and GradualResize × for the split case) before the termination check. Thread.

AI Review Tests — 04039 did not distinguish threshold / G from the unscaled behaviour. Correct, and now measured. The threshold is 400000 over the 1,000,000-row fixture with 4 groups of 4 outputs: about 250k rows per group, so the global value is never reached by a group while the divided one (100000) is crossed in every group. The observable is the runtime one from processors_profile_log: with the division 16 of 16 AggregatingTransform receive rows, without it (simulated by raising the global threshold by the group count) only 5 — 5 rather than 4 because the deadlock-avoidance branch promotes one waiting output, which is why the assertion is "more than two per group". Thread.

The one red on a61fdc38d865 is unrelated: AST fuzzer (amd_release, oracle) failed with a TLP aggregate oracle mismatch on SELECT count() FROM tab__fuzz_29 WHERE hasPhrase(val, 'the the') — a hasPhrase wrong-result, tracked by #117075 with the fix in flight in #119367. That query has no GROUP BY at all, and the fuzzer run had min_rows_per_stream_for_gradual_resize = 0 / min_bytes_per_stream_for_gradual_resize = 0, so it never reached this feature's code path.

The cloud benchmark verdict is still based on a false premise ("on by default"): both thresholds default to 0 since bde58b368549, and with both at 0 use_gradual_resize is false and the pipeline is byte-identical to master, so the flagged ClickBench and TPC-H queries ran the unchanged strict-resize path. A benchmark of the feature has to set one of the thresholds; that run on the current head is still owed by @alexey-milovidov, and it is what the changelog claim should rest on.

Verified on a server built from this head: all *gradual_resize* stateless tests pass (05025 and 05056 fail only on this private server, which has no test_shard_localhost cluster), 04039 and 04262 pass 3/3, and the 139 query-plan/aggregation/serialization unit tests pass, including the two AggregatingStep ones.

The draft status stays, and the two reserved design questions (locality of the ramp-up, projection parity) are untouched.

A storage that reports `hasEvenlyDistributedRead` already spreads the rows over the reading streams, so `AggregatingStep::transformPipeline` skips the pre-aggregation resize entirely and the gradual/strict choice never arises locally. The query plan serialization does not carry that property: every deserialized `AggregatingStep` is reconstructed with `storage_has_evenly_distributed_read` unset, so a shipped plan fragment does reach the resize branch, and the serialized `gradual_resize_enabled` bit made it build a `GradualResize` that the same query never builds when it is planned locally - the pipeline shape would depend on the transport rather than on the source contract. The bit is now left off the wire for such a step, so a shipped plan keeps the strict resize that is built there today. New test `05182_gradual_resize_evenly_distributed_read_shipped_plan` pins both directions over `serialize_query_plan = 1`: a `Memory` source (evenly distributed read) builds no `GradualResize` on the shard, while the same shipped-plan shape over a `MergeTree` source still does. Verified against a control binary without the fix, where the first assertion returns `1`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

alexey-milovidov · 2026-09-12T08:33:43Z

🕵 Round 308 (head 4ecefaef0356, previous 0bf8056a0fdb): both new AI Review findings addressed. CI of 0bf8056a0fdb had no failures (everything completed was green; a few stateless lanes and CH Inc sync were still running), and the branch is only 11 commits behind master, so no merge this round.

Shipped plan over an evenly distributed read (4ecefaef0356). The finding is real: deserialize reconstructs every AggregatingStep with storage_has_evenly_distributed_read unset (the pre-existing TODO), so a fragment shipped under serialize_query_plan = 1 reaches the pre-aggregation resize branch even for a source such as Memory or numbers_mt, where a locally planned query builds no resize at all — and the serialized gradual_resize_enabled bit made it build a GradualResize there. The bit is now left off the wire when the step has an evenly distributed read, so the shipped plan keeps the strict resize that is built there today and the processor choice no longer depends on the transport. Restoring the property itself into the deserialized step would change the strict path too; that remains the TODO.

New test 05182_gradual_resize_evenly_distributed_read_shipped_plan asserts the shard-side processors through processors_profile_log: 0 GradualResize for the Memory source, 1 for the same shipped-plan shape over MergeTree (positive control). Verified against a control binary built without the fix, where the first assertion comes back 1. Details in the thread.

Current-head benchmark. Measured with both thresholds at 0 (byte-identical to master) against one threshold enabled, on the current head. The only improvement I could reproduce is ≈ 3 % on 600000 rows / 300000 keys with heavy state at max_threads = 64; every other shape I tried was neutral or slower, up to ≈ 2× when the threshold holds the aggregation at one stream for most of the scan. Full table in the thread. This is a single 96-core machine with sub-second queries, so it is evidence, not a verdict: the rollout decision, the default thresholds and the cloud benchmark on the intended workload stay with @alexey-milovidov.

Local verification: private server on a non-default port, all 8 *gradual_resize* tests pass (04039, 04262, 04894, 05025, 05055, 05056, 05099, 05182), and the serialize_query_plan group passes except 04043_materialized_cte_serialize_query_plan, which needs the two-shard cluster the minimal local server does not have.

The pull request stays a draft: the steady-state locality trade-off and the projection-path parity remain design calls for @alexey-milovidov.

…essor

The per-group threshold-scaling case asserts how many `AggregatingTransform` were fed rows by the split `GradualResize` groups. Its threshold arithmetic assumes that the whole 1000000-row fixture flows through one local pre-aggregation pipeline: with `max_threads = 16` and `min_outstreams_per_resize_after_split = 4` each of the 4 groups must see about 250000 rows, so that only the per-group threshold (100000) can fire and not the global one (400000). With parallel replicas the rows are spread over the replicas, so a single group sees well below 100000 and never activates its remaining outputs - the assertion then reports `4\t0` instead of `4\t1`, reproducibly, on `Stateless tests (amd_llvm_coverage, ParallelReplicas, s3 storage, parallel)`. Pin `enable_parallel_replicas = 0` for the test: every assertion in it is about the shape of, and the number of rows pushed through, the local pre-aggregation pipeline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… text Two actions required by the AI review of `4ecefaef0356`: - `min_outstreams_per_resize_after_split` described itself as applying only to `Resize` and `StrictResize`, but `Pipe::resizeGradual` now reuses the same split path, so the description contradicted the two new `*_for_gradual_resize` settings. It now names `GradualResize` too and points at the threshold division among the split groups. - The `SettingsChangesHistory` entry for `min_rows_per_stream_for_gradual_resize` claimed the setting improves "performance on small data volumes". The benchmark measured on the current head supports no such blanket claim (one narrow shape gains about 3%, other shapes are neutral or slower when the threshold holds the pipeline at too few streams), so this user-facing release text is now a factual capability description that also states the setting is disabled by default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…essor # Conflicts: # src/Core/ProtocolDefines.h

The descriptions of `min_rows_per_stream_for_gradual_resize` and `min_bytes_per_stream_for_gradual_resize` read as if any keyed `GROUP BY` could take the gradual path, but `AggregatingStep::transformPipeline` consults the thresholds only when it builds the hash-based pre-aggregation resize stage. Aggregation in order of the sorting key, storages with evenly distributed reads (`Memory`, `system.numbers`), aggregation over independent partitions and single-stream reads never reach that branch, so the settings are silently ignored there. Both descriptions now state the contract as "only when the planner builds the hash-based pre-aggregation resize stage" and enumerate those bypasses next to the already documented ones. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

alexey-milovidov · 2026-09-13T10:11:36Z

🕵 Merged master (the branch was 707 commits behind and CONFLICTING: master had taken query-plan serialization versions 17 and 18, so this PR's second AggregatingStep flags byte moved from 17 to 19) and adopted the remaining AI Review Major in 0503bb8 (both setting descriptions now state exactly where the thresholds apply, see the thread reply). All 8 gradual-resize tests and the resize / plan-serialization selectors pass locally on the merged branch.

The sole red on the previous head, Upgrade check (amd_release) with Logical error: Virtual row has different type for CAST(__table1.json.b, 'String'_String), is unrelated to this PR and fleet-wide (57 PRs on 2026-09-11, 23 on 2026-09-12, 7 on 2026-09-13). It is raised by the previous release's 26.8 server during the pre-upgrade stress load, when the randomizer added by #116783 sets cast_keep_nullable = 1 without the not upgrade_check guard, so #119385 on master cannot clear it. Fix in a separate PR: #119768.

…ade load Since ClickHouse#116783 the stress randomization enables `cast_keep_nullable = 1` in a third of the runs, and, unlike its sibling arms, without the `not upgrade_check` guard. The upgrade check runs that load against the previous release's server (26.8.2.7), which predates ClickHouse#119385: it matches the sorting key `CAST(json.b, 'String')` to the same expression in `ORDER BY` by name and arity only, although under `cast_keep_nullable = 1` the query types it `Nullable(String)` while the key is `String`. Read-in-order with `read_in_order_use_virtual_row = 1` then aborts the shipped 26.8 server in `setVirtualRow` with `Logical error: Virtual row has different type` while running `03277_json_subcolumns_in_primary_key`, so `Upgrade check (amd_release)` went red on dozens of unrelated pull requests (57 on 2026-09-11, 23 on 2026-09-12, 7 on 2026-09-13). A master fix cannot clear it, because the exception is raised by the old binary. Guard the arm like the sibling `serialize_query_plan` arm (5620afa). CI: https://s3.amazonaws.com/clickhouse-test-reports/praktika.html?PR=99495&sha=c84da07b7cc18cc399ef06b7fc656f99a72b713a&name_0=PR&name_1=Upgrade%20check%20(amd_release) PR: ClickHouse#99495 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…ade load Since #116783 the stress randomization enables `cast_keep_nullable = 1` in a third of the runs, and, unlike its sibling arms, without the `not upgrade_check` guard. The upgrade check runs that load against the previous release's server (26.8.2.7), which predates #119385: it matches the sorting key `CAST(json.b, 'String')` to the same expression in `ORDER BY` by name and arity only, although under `cast_keep_nullable = 1` the query types it `Nullable(String)` while the key is `String`. Read-in-order with `read_in_order_use_virtual_row = 1` then aborts the shipped 26.8 server in `setVirtualRow` with `Logical error: Virtual row has different type` while running `03277_json_subcolumns_in_primary_key`, so `Upgrade check (amd_release)` went red on dozens of unrelated pull requests (57 on 2026-09-11, 23 on 2026-09-12, 7 on 2026-09-13). A master fix cannot clear it, because the exception is raised by the old binary. Guard the arm like the sibling `serialize_query_plan` arm (5620afa). CI: https://s3.amazonaws.com/clickhouse-test-reports/praktika.html?PR=99495&sha=c84da07b7cc18cc399ef06b7fc656f99a72b713a&name_0=PR&name_1=Upgrade%20check%20(amd_release) PR: #99495 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 6b1566c)

alexey-milovidov · 2026-09-13T23:18:28Z

🕵 Status on head 0503bb862d7b: public CI is fully green (162 checks passed, 0 failed), there are no unresolved review threads, and the AI Review verdict is "no new blockers or majors". The only pending item was CH Inc sync, which had sat pending with an empty target URL since 2026-09-13T10:38Z because the private sync PR (clickhouse-private#52465) was frozen at a 2026-08-29 tip and reported CONFLICTING, so the bot could not advance it.

Repaired the private sync-upstream/pr/99495 branch by hand: merged private master first (one conflict in 02531_two_level_aggregation_bug.sh, resolved to the public head's copy since private and public master are byte-identical there), then merged this head (one conflict in SettingsChangesHistory.cpp, a duplicated pair of the *_for_gradual_resize entries, dropped). The private-master-to-head delta matches this PR's delta exactly (36 files, +1826/-24), and there is no plan-serialization version collision (master is still at 18, this PR takes 19). The sync PR is now MERGEABLE; the public status will refresh when private CI finishes. Nothing to push on the public branch this round.

alexey-milovidov · 2026-09-14T19:07:37Z

As the settings are disabled by default, why do we see any performance difference in performance tests?

alexey-milovidov · 2026-09-14T19:09:21Z

Now the question - is there any value in having these fine-tuning settings without any changes by default?

clickhouse-gh Bot added the pr-performance Pull request with some performance improvements label Mar 14, 2026

alexey-milovidov requested a review from nihalzp March 14, 2026 07:36

clickhouse-gh Bot reviewed Mar 14, 2026

View reviewed changes

Comment thread src/Processors/ResizeProcessor.cpp Outdated

nihalzp self-assigned this Mar 14, 2026

alexey-milovidov and others added 2 commits March 15, 2026 02:31

Merge master into gradual-resize-processor

8afaad2

clickhouse-gh Bot reviewed Mar 16, 2026

View reviewed changes

Comment thread src/Processors/QueryPlan/AggregatingStep.cpp

nickitat mentioned this pull request Mar 16, 2026

Fix negative scaling for short queries with aggregation #99493

Merged

1 task

alexey-milovidov and others added 2 commits March 16, 2026 21:50

Merge fix for flaky 04004_alter_modify_column_ttl_without_type test

6440176

clickhouse-gh Bot reviewed Mar 17, 2026

View reviewed changes

Comment thread src/Processors/ResizeProcessor.cpp Outdated

Merge branch 'master' into gradual-resize-processor

96cf699

clickhouse-gh Bot reviewed Mar 17, 2026

View reviewed changes

Comment thread .claude/learnings.md

Merge branch 'master' into gradual-resize-processor

48aee06

alexey-milovidov mentioned this pull request Mar 18, 2026

Fix broken docker pull retry in integration tests #99828

Merged

1 task

alexey-milovidov added 2 commits March 17, 2026 22:25

Merge branch 'master' into gradual-resize-processor

1c26b99

Merge master into gradual-resize-processor

d56fafb

Merge branch 'master' into gradual-resize-processor

583789e

clickhouse-gh Bot reviewed Mar 19, 2026

View reviewed changes

Comment thread src/Processors/ResizeProcessor.cpp Outdated

alexey-milovidov and others added 3 commits March 19, 2026 07:25

Merge branch 'master' into gradual-resize-processor

a280647

Merge remote branch and resolve conflicts in favor of demand-driven i…

89608a8

…mplementation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

alexey-milovidov and others added 2 commits September 12, 2026 02:48

Merge remote-tracking branch 'origin/master' into gradual-resize-proc…

fccd727

…essor # Conflicts: # src/Core/ProtocolDefines.h # src/Core/SettingsChangesHistory.cpp

clickhouse-gh Bot reviewed Sep 12, 2026

View reviewed changes

Comment thread src/Processors/QueryPlan/AggregatingStep.cpp

Comment thread src/Core/SettingsChangesHistory.cpp Outdated

clickhouse-gh Bot added comp-query-execution Runtime execution pipeline, processors, vectorized operators, resource management during execution. and removed comp-aggregation GROUP BY / aggregation framework (hash tables, spilling, external aggregation). labels Sep 12, 2026

clickhouse-gh Bot reviewed Sep 12, 2026

View reviewed changes

Comment thread src/Core/Settings.cpp

clickhouse-gh Bot added comp-aggregation GROUP BY / aggregation framework (hash tables, spilling, external aggregation). and removed comp-query-execution Runtime execution pipeline, processors, vectorized operators, resource management during execution. labels Sep 12, 2026

alexey-milovidov and others added 3 commits September 12, 2026 14:49

Merge remote-tracking branch 'origin/master' into gradual-resize-proc…

860499e

…essor

clickhouse-gh Bot reviewed Sep 12, 2026

View reviewed changes

Comment thread src/Core/Settings.cpp Outdated

alexey-milovidov and others added 2 commits September 13, 2026 09:13

Merge remote-tracking branch 'origin/master' into gradual-resize-proc…

44c251d

…essor # Conflicts: # src/Core/ProtocolDefines.h

alexey-milovidov mentioned this pull request Sep 13, 2026

Do not randomize cast_keep_nullable in the upgrade check's pre-upgrade load #119768

Merged

Uh oh!

Conversation

alexey-milovidov commented Mar 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

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

Uh oh!

clickhouse-gh Bot commented Mar 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI Review

Summary

Final Verdict

LLVM Coverage Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nihalzp commented Mar 17, 2026

Uh oh!

alexey-milovidov commented Mar 18, 2026

Uh oh!

Uh oh!

alexey-milovidov commented Sep 12, 2026

Uh oh!

Uh oh!

Uh oh!

alexey-milovidov commented Sep 12, 2026

Uh oh!

Uh oh!

Uh oh!

alexey-milovidov commented Sep 13, 2026

Uh oh!

alexey-milovidov commented Sep 13, 2026

Uh oh!

alexey-milovidov commented Sep 14, 2026

Uh oh!

alexey-milovidov commented Sep 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

alexey-milovidov commented Mar 14, 2026 •

edited

Loading

clickhouse-gh Bot commented Mar 14, 2026 •

edited

Loading