Skip to content

Grouped approx_distinct can overflow Arrow BinaryArray state offsets #24072

Description

@sesteves

Bug

Grouped approx_distinct can panic while DataFusion materializes intermediate HyperLogLog states. In DataFusion 54.1.0, each HLL state contains 16,384 one-byte registers but is represented as ScalarValue::Binary and declared as Arrow DataType::Binary.

GroupsAccumulatorAdapter::state collects one scalar state per group and calls ScalarValue::iter_to_array. Arrow BinaryArray uses signed 32-bit offsets, so this deterministic boundary is unrepresentable:

131,072 groups * 16,384 bytes = 2,147,483,648 bytes

The resulting panic is:

byte array offset overflow

The stack reaches GroupedHashAggregateStream::emit -> GroupsAccumulatorAdapter::state -> ScalarValue::iter_to_array.

Spill does not prevent it

The branch-54 grouped hash aggregate spill path first calls emit(EmitTo::All, true), so all current group states are materialized in one record batch before spill can write them. Terminal output similarly materializes EmitTo::All and only slices the resulting batch afterward.

This means the panic can occur while trying to spill, and memory pressure is not applied before a potentially multi-gigabyte temporary state array is built.

Suggested direction

  1. Represent dense fixed-width HLL register state without cumulative variable-width offsets, such as FixedSizeBinary(16384) for the branch-54 implementation.
  2. Materialize terminal and spill aggregate state in bounded chunks rather than building EmitTo::All first.
  3. Propagate memory or spill exhaustion as DataFusionError::ResourcesExhausted rather than unwinding.

A direct 131,072-group regression requires about 2 GiB per HLL expression, so practical CI coverage can verify the fixed-size state schema and use a small configured emission-byte threshold or synthetic fixed-size accumulator to prove bounded draining.

Related work

Newer DataFusion has evolved its grouped HLL implementation, including sparse state, so the exact state representation on main may need a different design. The invariant remains that dense HLL state and grouped spill/output must not depend on one variable-width Arrow array with 32-bit cumulative offsets.

Activity

  1. saadtajwar commented on Aug 3, 2026

    @saadtajwar
    Contributor

    Hi @sesteves , happy to help out here unless you're going to take this one! Thanks!

  2. 2010YOUY01 commented on Aug 4, 2026

    @2010YOUY01
    Contributor

    To solve this issue in aggregate, we need #7065

    However (at least in the near term), we can't guarantee to prevent similar issue in other operators that buffer and concat input batches: BianryArray will overflow if it's > 2GB, there are other places like hash join might trigger the similar bug.

    I think we could let it return a better error message, and suggest user to use LargeBinary type instead.

  3. sesteves commented on Aug 5, 2026

    @sesteves
    ContributorAuthor

    @saadtajwar Feel free to take this one! I appreciate the help

  4. saadtajwar commented on Aug 7, 2026

    @saadtajwar
    Contributor

    Thank you @sesteves - I'll finish up the few issues I'm working on now then take if it's still available in the next few days :) if anyone sees this issue and wants to pick it up before I've assigned it to myself then please feel free to!

  5. saadtajwar commented on Aug 9, 2026

    @saadtajwar
    Contributor

    take

  6. Rich-T-kid commented on Aug 17, 2026

    @Rich-T-kid
    Contributor

    This seems very similar to #23127.

    i'm curious on opinions on allowing certain datatypes in data fusion to 'grow' for example utf8/binary -> largeutf8/largebinary or dict<i8,_> -> dict<i16,_>. currently schemas are immutable and changing this would require a lot of effort.
    like @2010YOUY01 mentioned

    I think we could let it return a better error message, and suggest user to use LargeBinary type instead.

    is the best current option but ideally we could handle this for users

  7. alamb commented on Aug 27, 2026

    @alamb
    Contributor

    I filed a ticket / epic to track the idea of blocked state management here

    Depending on how it is implemented, we may be able to use t for approx_distinct too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions