Validate block_size_bytes parameter in estimateCompressionRatio by alexey-milovidov · Pull Request #100298 · ClickHouse/ClickHouse

alexey-milovidov · 2026-03-21T19:08:15Z

The AST fuzzer found that passing a huge value like 9223372036854775806 as the block_size_bytes parameter to estimateCompressionRatio causes a LOGICAL_ERROR exception when the CompressedWriteBuffer constructor tries to allocate a buffer of that size.

Add a maximum limit of 256 MiB for the block_size_bytes parameter and reject larger values with a proper error message.

AST fuzzer report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?REF=master&sha=ccdc901a930a375ee446b7625ccf22c93732350e&name_0=MasterCI&name_1=AST%20fuzzer%20%28arm_asan_ubsan%29

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 LOGICAL_ERROR exception in estimateCompressionRatio when block_size_bytes parameter is extremely large.

Documentation entry for user-facing changes

Documentation is written (mandatory for new features)

The AST fuzzer found that passing a huge value like 9223372036854775806 as the `block_size_bytes` parameter to `estimateCompressionRatio` causes a LOGICAL_ERROR exception when trying to allocate a buffer of that size. Add a maximum limit of 256 MiB for the parameter and reject larger values with a proper error message. https://s3.amazonaws.com/clickhouse-test-reports/json.html?REF=master&sha=ccdc901a930a375ee446b7625ccf22c93732350e&name_0=MasterCI&name_1=AST%20fuzzer%20%28arm_asan_ubsan%29 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

clickhouse-gh · 2026-03-21T19:08:56Z

Workflow [PR], commit [0daa483]

Summary: ✅

AI Review

Summary

This PR adds validation for estimateCompressionRatio block_size_bytes to prevent oversized allocations and adds a focused stateless test for overflow/boundary inputs. I found no new high-confidence issues beyond concerns already raised in existing inline comments.

Missing context

⚠️ Several CI checks are still pending, so final confidence in cross-platform/regression coverage is limited.

ClickHouse Rules

Item	Status	Notes
Deletion logging	➖
Serialization versioning	➖
Core-area scrutiny	✅
No test removal	✅
Experimental gate	➖
No magic constants	✅
Backward compatibility	✅
`SettingsChangesHistory.cpp`	➖
PR metadata quality	✅
Safe rollout	✅
Compilation time	✅

Final Verdict

Status: ✅ Approve

alexey-milovidov

LGTM

…size

The test was flaky under ASan because `clickhouse-local` emits the warning "ASan doesn't fully support makecontext/swapcontext functions" to stderr. Test cases test2 and test4 already filtered this warning with `grep -av`, but test1 and test3 did not, causing the `awk` to produce extra garbage output like "ASandoesn't" and fail the diff. Add the same ASan warning filter to test1 and test3. #100298 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The `transform` function casts WHEN values to the expression type. When WHEN values are Nullable (e.g. `CAST(NULL AS Nullable(Int32))`) but the expression is non-Nullable, the cast fails with "Cannot convert NULL value to non-Nullable type". This became visible after #99839 made `aggregate_functions_null_for_empty` produce Nullable result types for more aggregates, causing SQLLogic tests to hit this path. Fix by wrapping the entire `transform` attempt (array construction, build, and execute) in a try-catch that falls through to the `multiIf` path on type-related errors. The `multiIf` path handles Nullable WHEN values correctly via `caseWhenEquals`. https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=100293&sha=f067c3bec9fe4a7a07e810a27c9587ea9f777bfb&name_0=PR&name_1=SQLLogic%20test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Use `tryGetLeastSupertype` and a Nullable compatibility check to determine whether `transform` can handle the types, rather than catching multiple exception codes after the fact. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

clickhouse-gh · 2026-03-22T18:46:11Z

src/AggregateFunctions/AggregateFunctionEstimateCompressionRatio.cpp

+                throw Exception(ErrorCodes::BAD_QUERY_PARAMETER, "block_size_bytes should be greater than 0");
+
+            /// Limit to 256 MiB to prevent absurd memory allocations from fuzzed queries
+            static constexpr UInt64 max_block_size_bytes = 256 * 1024 * 1024;


⚠️ max_block_size_bytes = 256 * 1024 * 1024 is introduced as a hidden magic constant. This changes user-visible behavior for estimateCompressionRatio without making the limit discoverable/configurable, and it can drift from existing compression-related limits (max_compress_block_size).

Please either:

reuse an existing setting/constant for compression block bounds, or

surface this limit explicitly in function documentation and keep the bound in a shared named constant with rationale.

This is okay.

alexey-milovidov · 2026-03-22T22:16:08Z

#100384

…size

clickhouse-gh · 2026-03-23T23:47:30Z

tests/queries/0_stateless/04053_estimate_compression_ratio_huge_block_size.sql

@@ -0,0 +1,2 @@
+SELECT estimateCompressionRatio(9223372036854775806)(*) FROM numbers(1); -- { serverError BAD_QUERY_PARAMETER }
+SELECT estimateCompressionRatio(268435457)(*) FROM numbers(1); -- { serverError BAD_QUERY_PARAMETER }


⚠️ Please add a boundary-acceptance check for block_size_bytes = 268435456 (exactly 256 MiB).

This test currently only covers invalid values (> 256 MiB), so an off-by-one regression in the upper bound would not be detected.

No, this does not matter.

clickhouse-gh · 2026-03-26T23:53:08Z

tests/queries/0_stateless/04054_caseWithExpression_nullable_when.sql

@@ -0,0 +1,27 @@
+-- Test that CASE expression handles Nullable WHEN values gracefully


⚠️ This test file appears unrelated to this PR's stated fix (estimateCompressionRatio block_size_bytes validation). The PR description and code changes here do not explain the CASE/Nullable behavior coverage added in 04054.

Please either:

split these tests into a separate PR with matching motivation, or

update this PR description to explain why this behavior must be validated together with this fix.

tiandiwonder · 2026-03-27T01:08:43Z

src/AggregateFunctions/AggregateFunctionEstimateCompressionRatio.cpp

"... The default value is 1 MiB (1048576 bytes). Maximum allowed value is 256 MiB (268435456 bytes)."

…size

clickhouse-gh · 2026-03-28T23:00:22Z

tests/queries/0_stateless/04054_caseWithExpression_nullable_when.sql

+SELECT CASE 10 WHEN NULL THEN 1 WHEN 10 THEN 2 ELSE 3 END;
+
+-- With cast_keep_nullable, CAST(NULL AS INTEGER) becomes Nullable
+SET cast_keep_nullable = 1;


⚠️ This test toggles cast_keep_nullable but never restores it. Since settings persist within the test file, all subsequent CASE checks run under a modified session state, which makes the coverage less isolated and can hide regressions under default settings.

Please add SET cast_keep_nullable = 0; (or move the SET immediately before/after the single query that needs it) so later checks validate behavior independently.

…ssion-ratio-huge-block-size

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

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

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

clickhouse-gh · 2026-03-29T09:17:23Z

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	84.20%	84.20%	+0.00%
Functions	24.60%	24.60%	+0.00%
Branches	76.70%	76.80%	+0.10%

Changed lines: 100.00% (18/18) · Uncovered code

Full report · Diff report

MergeTree settings like max_compress_block_size, min_compress_block_size, marks_compress_block_size, and primary_key_compress_block_size flow directly to buffer allocation constructors (CompressedWriteBuffer, WriteBufferFromFile) without size validation. When the AST fuzzer mutates these to values near INT64_MAX via CREATE TABLE SETTINGS, adding SIMD padding (~63 bytes) overflows past the allocator's checkSize() limit of 2^63, causing a LOGICAL_ERROR abort (server crash). Fix: clamp all compress block size settings to 256 MiB (matching the limit in PR ClickHouse#100298 for estimateCompressionRatio) in the MergeTreeWriterSettings constructor and the column-level override in MergeTreeDataPartWriterWide. Affected crash variants: STID 0883-4864, 0883-4856, 0883-5a5b, 0883-2066, 0883-22d2, 0883-45dc (24 hits in 30 days, 3 on master). The estimateCompressionRatio variant (0883-248c) was separately fixed by PR ClickHouse#100298. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…pression-ratio-huge-block-size Validate block_size_bytes parameter in estimateCompressionRatio

clickhouse-gh bot added the pr-bugfix Pull request with bugfix, not backported by default label Mar 21, 2026

alexey-milovidov commented Mar 21, 2026

View reviewed changes

alexey-milovidov added 2 commits March 21, 2026 20:12

Merge branch 'master' into fix-estimate-compression-ratio-huge-block-…

dd6f788

…size

Merge branch 'master' into fix-estimate-compression-ratio-huge-block-…

7a1bada

…size

alexey-milovidov and others added 3 commits March 22, 2026 18:21

Merge master into fix-estimate-compression-ratio-huge-block-size

15a558f

clickhouse-gh bot reviewed Mar 22, 2026

View reviewed changes

alexey-milovidov mentioned this pull request Mar 22, 2026

Fix inconsistent AST formatting for GROUP BY CUBE(...) WITH ROLLUP #100376

Open

1 task

alexey-milovidov force-pushed the fix-estimate-compression-ratio-huge-block-size branch from bb6c905 to 49fd1a4 Compare March 22, 2026 23:05

Merge branch 'master' into fix-estimate-compression-ratio-huge-block-…

c7b0783

…size

clickhouse-gh bot reviewed Mar 23, 2026

View reviewed changes

alexey-milovidov added 2 commits March 24, 2026 14:58

Merge master into fix-estimate-compression-ratio-huge-block-size

421e2b6

Merge master into fix-estimate-compression-ratio-huge-block-size

d1ac6aa

alexey-milovidov force-pushed the fix-estimate-compression-ratio-huge-block-size branch from d54605f to d1ac6aa Compare March 26, 2026 23:43

clickhouse-gh bot reviewed Mar 26, 2026

View reviewed changes

tiandiwonder self-assigned this Mar 27, 2026

tiandiwonder approved these changes Mar 27, 2026

View reviewed changes

Merge branch 'master' into fix-estimate-compression-ratio-huge-block-…

3b6526d

…size

clickhouse-gh bot reviewed Mar 28, 2026

View reviewed changes

alexey-milovidov and others added 4 commits March 29, 2026 00:23

Merge remote-tracking branch 'origin/master' into fix-estimate-compre…

9adf43b

…ssion-ratio-huge-block-size

Address review: document maximum block_size_bytes value in function…

ae44479

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

Address review: restore cast_keep_nullable after test to improve is…

6c06de7

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

Address review: add boundary acceptance test for exactly 256 MiB `blo…

0daa483

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

alexey-milovidov merged commit c29814f into master Mar 29, 2026
152 of 153 checks passed

alexey-milovidov deleted the fix-estimate-compression-ratio-huge-block-size branch March 29, 2026 16:01

robot-ch-test-poll added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 29, 2026

groeneai mentioned this pull request Mar 30, 2026

Clamp extreme compress block sizes to prevent allocator crash #101159

Open

Desel72 pushed a commit to Desel72/ClickHouse that referenced this pull request Mar 30, 2026

Merge pull request ClickHouse#100298 from ClickHouse/fix-estimate-com…

de292f1

…pression-ratio-huge-block-size Validate block_size_bytes parameter in estimateCompressionRatio

azat mentioned this pull request Mar 31, 2026

Logical error: Too large size (A) passed to allocator. It indicates an error. (STID: 0883-2066) #101172

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validate block_size_bytes parameter in estimateCompressionRatio#100298

Validate block_size_bytes parameter in estimateCompressionRatio#100298
alexey-milovidov merged 14 commits intomasterfrom
fix-estimate-compression-ratio-huge-block-size

alexey-milovidov commented Mar 21, 2026

Uh oh!

clickhouse-gh bot commented Mar 21, 2026 •

edited

Loading

Uh oh!

alexey-milovidov left a comment

Uh oh!

clickhouse-gh bot Mar 22, 2026

Uh oh!

alexey-milovidov Mar 24, 2026

Uh oh!

alexey-milovidov commented Mar 22, 2026

Uh oh!

clickhouse-gh bot Mar 23, 2026

Uh oh!

alexey-milovidov Mar 24, 2026

Uh oh!

clickhouse-gh bot Mar 26, 2026

Uh oh!

tiandiwonder Mar 27, 2026

Uh oh!

clickhouse-gh bot Mar 28, 2026

Uh oh!

clickhouse-gh bot commented Mar 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,2 @@
		SELECT estimateCompressionRatio(9223372036854775806)(*) FROM numbers(1); -- { serverError BAD_QUERY_PARAMETER }
		SELECT estimateCompressionRatio(268435457)(*) FROM numbers(1); -- { serverError BAD_QUERY_PARAMETER }

		@@ -0,0 +1,27 @@
		-- Test that CASE expression handles Nullable WHEN values gracefully

Conversation

alexey-milovidov commented Mar 21, 2026

Changelog category (leave one):

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

Documentation entry for user-facing changes

Uh oh!

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

Uh oh!

AI Review

Summary

Missing context

ClickHouse Rules

Final Verdict

Uh oh!

alexey-milovidov left a comment

Choose a reason for hiding this comment

Uh oh!

clickhouse-gh bot Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

alexey-milovidov Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

alexey-milovidov commented Mar 22, 2026

Uh oh!

clickhouse-gh bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

alexey-milovidov Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

clickhouse-gh bot Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

tiandiwonder Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

clickhouse-gh bot Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

clickhouse-gh bot commented Mar 29, 2026

LLVM Coverage Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

clickhouse-gh bot commented Mar 21, 2026 •

edited

Loading