Skip to content

Backport/stable 25.8/92588 - #2254

Open
VighneshPath wants to merge 3 commits into
Altinity:stable-25.8from
VighneshPath:backport/stable-25.8/92588
Open

Backport/stable 25.8/92588#2254
VighneshPath wants to merge 3 commits into
Altinity:stable-25.8from
VighneshPath:backport/stable-25.8/92588

Conversation

@VighneshPath

@VighneshPath VighneshPath commented Aug 24, 2026

Copy link
Copy Markdown

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 to CHANGELOG.md):

Fix potential crash caused by in place mutation of underlying const PREWHERE columns. This could've happened at column shrinking (IColumn::shrinkToFit) or filtering (IColumn::filter), which could've triggered concurrently from several threads. Backport of ClickHouse#92588.

Documentation entry for user-facing changes

Fix potential crash caused by in place mutation of underlying const PREWHERE columns. This could've happened at column shrinking (IColumn::shrinkToFit) or filtering (IColumn::filter), which could've triggered concurrently from several threads. Backport of ClickHouse#92588.

CI/CD Options

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Performance tests
  • Aarch64 tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All Regression
  • Disable CI Cache

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • CAS (content-addressed storage; Antalya only)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • OAuth (5m)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

… concurrent in-place mutations

ColumnNullable::shrinkToFit() now shrinks the null-map column via its
own shrinkToFit() (instead of reaching into its raw data array
directly), and MergeTreeReadTask::read() clones shared sub-columns via
IColumn::mutate() before shrinking -- the previous use_count()==1 guard
only checked the top-level column's refcount, missing the case where a
column (e.g. ColumnNullable) is uniquely owned per-thread but wraps a
nested column still shared with the query-wide ActionsDAG. The guard
itself is kept as a cheap early-out for columns still shared at the top
level; IColumn::mutate() handles the deeper, per-subcolumn sharing check
that this fix actually targets.

Matches upstream commit 693a065 for
both src/Columns/ColumnNullable.cpp and
src/Storages/MergeTree/MergeTreeReadTask.cpp exactly.

The MergeTreeRangeReader.cpp hunk from the upstream PR is not included:
this branch has no canInplaceFilter()/assumeMutable()->filter() fast
path, so that particular crash vector doesn't exist here.
…ToFit crash

Adds 03681_mergetree_shrink_const_nested_nullable, targeting the case
where a constant inside PREWHERE, wrapped in
materialize(toNullable(materialize(...))), produces a ColumnNullable
with use_count() == 1 at the top level but a shared nested column
underneath -- the case the old use_count()==1 guard in
MergeTreeReadTask::read() missed.

Verified: reverting the ClickHouse#92588 backport fix reproduces a SIGSEGV in
jemalloc (eset_remove/extent_recycle) within seconds of running this
test; with the fix in place, the test passes reliably.
Adds 03682_mergetree_shrink_const_array_float64, covering a second
variant of the ClickHouse#90695/ClickHouse#92588 class of bug via ColumnArray instead of
ColumnNullable: arrayMap's lambda body evaluates a bare Float64
literal over a single-element array, so ColumnConst::convertToFullColumn()'s
`if (s == 1) return data;` shortcut hands out a shared literal column
as the arrayMap result's nested data, wrapped in a fresh ColumnArray
with top-level use_count() == 1. The old use_count()==1 guard in
MergeTreeReadTask::read() missed the shared nested column here too.

Verified: crashes reliably (SIGSEGV) against an unpatched 25.8 build
within 30s; passes cleanly with the ClickHouse#92588 backport applied, which
fixes this case too since IColumn::mutate() recurses into ColumnArray's
subcolumns generically via forEachMutableSubcolumn(), not just
ColumnNullable's.
@VighneshPath
VighneshPath force-pushed the backport/stable-25.8/92588 branch from b2e5f8b to 007dd7b Compare August 25, 2026 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant