Fix segfault in external hash aggregate when radix bits grow after going external by carlopi · Pull Request #23757 · duckdb/duckdb · GitHub
Skip to content

Fix segfault in external hash aggregate when radix bits grow after going external - #23757

Merged
lnkuiper merged 2 commits into
duckdb:v1.5-variegatafrom
carlopi:segfault_investigation
Jul 14, 2026
Merged

lnkuiper merged 2 commits into
duckdb:v1.5-variegatafrom
carlopi:segfault_investigation

Conversation

@carlopi

@carlopi carlopi commented Jul 10, 2026 •

Copy link
Copy Markdown
Member

A thread's abandoned_data (the external-spill target) is sized once at the global radix bits of when it first went external and then frozen, but the global radix bits can keep growing afterwards along with the thread's own HT data. Merging the larger HT data into the smaller, stale abandoned_data makes RepartitionFinalizeStates compute NumberOfPartitions(new_radix_bits - old_radix_bits) with unsigned arithmetic: new < old underflows to a huge multiplier, indexing the partition and pin-state vectors out of bounds leading to segfault.

Fix: GrowAbandonedDataToRadixBits repartitions abandoned_data up to the current global radix bits (the supported new >= old direction) before merging into it, called in both the sink-phase external repartition and before the combine-phase Combine. As defense-in-depth, PartitionedTupleData::Repartition now throws instead of underflowing when asked to go to fewer partitions.

…ing external

A thread's abandoned_data (the external-spill target) is sized once at the
global radix bits of when it first went external and then frozen, but the
global radix bits can keep growing afterwards along with the thread's own HT
data. Merging the larger HT data into the smaller, stale abandoned_data makes
RepartitionFinalizeStates compute NumberOfPartitions(new_radix_bits -
old_radix_bits) with unsigned arithmetic: new < old underflows to a huge
multiplier, indexing the partition and pin-state vectors out of bounds ->
segfault.

Fix: GrowAbandonedDataToRadixBits repartitions abandoned_data up to the current
global radix bits (the supported new >= old direction) before merging into it,
called in both the sink-phase external repartition and before the combine-phase
Combine. As defense-in-depth, PartitionedTupleData::Repartition now throws
instead of underflowing when asked to go to fewer partitions.
@carlopi
carlopi requested a review from lnkuiper July 10, 2026 18:47
@carlopi

carlopi commented Jul 10, 2026 •

Copy link
Copy Markdown
Member Author

@carlopi
carlopi marked this pull request as draft July 10, 2026 19:07
@carlopi
carlopi marked this pull request as ready for review July 10, 2026 19:07
lance-duckdb's committed Cargo.lock pins ethnum 1.5.2, whose error.rs
fabricates a `core::num::TryFromIntError` via `mem::transmute(())`. That
assumed the type was zero-sized; it is now 8 bits, so on Rust >= 1.97 the
build fails with E0512. Rust 1.97 went stable on 2026-07-07, which is what
turned this into a CI failure here.

ethnum 1.5.3 replaces the transmutes with safe constructors. jsonb (the crate
that pulls ethnum in, via lance-arrow) requires `ethnum ^1.5.2`, so 1.5.3
already satisfies it: the fix is purely a lockfile refresh, a two-line diff
with no manifest change and no other dependency moving.

Carry that refresh as an extension patch so the build is unblocked without
waiting on upstream. The patch context is generated against the pinned
GIT_TAG cd592bc, so the tag and the patch must be bumped together.

The durable fix belongs upstream in lance-format/lance-duckdb, which has not
refreshed its lockfile; the sibling lance-format/lance repo already has.
@carlopi
carlopi force-pushed the segfault_investigation branch from e17bd69 to c781991 Compare July 12, 2026 07:25
@carlopi
carlopi marked this pull request as draft July 12, 2026 07:25
@carlopi
carlopi marked this pull request as ready for review July 12, 2026 07:25
@lnkuiper
lnkuiper merged commit d6d9cda into duckdb:v1.5-variegata Jul 14, 2026
114 checks passed
@carlopi
carlopi deleted the segfault_investigation branch July 14, 2026 09:13
kaka11chen pushed a commit to kaka11chen/astrovela_vane that referenced this pull request Aug 14, 2026
…ing external (duckdb/duckdb#23757)

A thread's `abandoned_data` (the external-spill target) is sized once at
the global radix bits of when it first went external and then frozen,
but the global radix bits can keep growing afterwards along with the
thread's own HT data. Merging the larger HT data into the smaller, stale
`abandoned_data` makes `RepartitionFinalizeStates` compute
`NumberOfPartitions(new_radix_bits - old_radix_bits)` with unsigned
arithmetic: new < old underflows to a huge multiplier, indexing the
partition and pin-state vectors out of bounds leading to segfault.

Fix: `GrowAbandonedDataToRadixBits` repartitions `abandoned_data` up to
the current global radix bits (the supported new >= old direction)
before merging into it, called in both the sink-phase external
repartition and before the combine-phase Combine. As defense-in-depth,
PartitionedTupleData::Repartition now throws instead of underflowing
when asked to go to fewer partitions.

Upstream-Commit: duckdb/duckdb@d6d9cda
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants