{{ message }}
Repository navigation
Fix segfault in external hash aggregate when radix bits grow after going external - #23757
Merged
Merged
Conversation
…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.
Member
Author
carlopi
marked this pull request as draft
July 10, 2026 19:07
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
force-pushed
the
segfault_investigation
branch
from
July 12, 2026 07:25
e17bd69 to
c781991
Compare
carlopi
marked this pull request as draft
July 12, 2026 07:25
carlopi
marked this pull request as ready for review
July 12, 2026 07:25
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
8 of 9 tasks
5 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

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, staleabandoned_datamakesRepartitionFinalizeStatescomputeNumberOfPartitions(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:
GrowAbandonedDataToRadixBitsrepartitionsabandoned_dataup 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.