[Nook001]

Which issue does this PR close?

Closes #26937. Related to #23393 and #25650; discovered while investigating #20942.

Rationale

DISTINCT aggregation can retain significantly more HashSet memory than it reports to DataFusion's MemoryPool. For example, an 8,192-row Float64 batch with 128 distinct values retains 147,472 bytes of heap while the original estimator reports 2,344 bytes, allowing queries to exceed their expected memory reservation.

Some nested accumulators also double-count embedded objects. Primitive COUNT(DISTINCT) additionally had a small residual error from capacity-based estimation.

Changes

• Use allocation_size() to account for GenericDistinctBuffer's retained HashSet heap and count the accumulator object once.
• Switch PrimitiveDistinctCountAccumulator to the existing datafusion_common::HashSet (hashbrown), keeping its explicit RandomState hasher, and account for allocations directly.
• Fix ownership accounting in DISTINCT COUNT/SUM/AVG, decimal AVG, DISTINCT variance, and the boxed dictionary COUNT wrapper.
• Add regression tests for capacity boundaries, nulls, overlapping merges, integer extremes and Decimal128/256, plus a 40-case update/merge Criterion benchmark.

SQL results and public APIs are unchanged.

Validation

Memory accounting

Independent Windows/MSVC and Rocky Linux allocator measurements each covered 41 scenarios × 5 accumulator types × 3 revisions × 50 repetitions = 30,750 observations. All 10,250 current-revision observations per platform had zero requested-retained-heap estimation error; the 615 scenario/type/revision summaries matched across platforms. Tests include small allocated tables, reservation boundaries, chunked updates, retained capacity after clear and cleanup on drop.

Example: 8,192 rows / 128 NDV (retained heap bytes, excluding the inline accumulator):

Type	Measured	Previous PR estimate	Current estimate
Generic Float64	147,472	147,472	147,472
Primitive Int64	147,472	147,456	147,472
Primitive Int32	81,936	81,920	81,936
Primitive Decimal128	278,544	278,528	278,544
Primitive Decimal256	540,688	540,672	540,688

The size() checks do not allocate; tested drop and low-memory SQL paths release their reservations.

Correctness and repository checks

• Windows: formatting, all-target/all-feature Clippy, 67 aggregate-common library tests, aggregate.slt and benchmark correctness checks passed.
• Rocky Linux: targeted aggregate-common tests (65 passed), relevant Clippy checks, allocator and restrictive-memory SQL tests passed in the local dependency-override setup.
• The extended Windows workspace run stopped after one test_tpch_unparser_roundtrip failure involving Q15 multi-statement planning (1,483 other core integration tests passed); the full workspace run remains incomplete.

Performance and current status

The pushed PR head 46af5e0a7 resolves the measured memory-accounting errors, but stock hashbrown 0.17.1 shows a repeatable regression in real nullable Int64 COUNT(DISTINCT) SQL on Rocky Linux (about 5.76 → 6.36 ms versus the previous std-HashSet revision). This PR therefore remains Draft.

An independently tested hashbrown-side candidate changes HashMap::insert from inline to inline(always) under its existing inline-more feature. Against the matching unmodified hashbrown build, it improves:

• Nullable Int64 SQL: 6.36 → 5.16 ms (−18.83%).
• Decimal report SQL: 12.83 → 10.89 ms (−15.07%).
• Unique Int64 SQL: 60.02 → 55.47 ms (−7.58%).
• 40 microbenchmark combinations: 1.79% lower equal-weight geometric mean time.

Independent paired runs confirmed the main SQL gains; the earlier large Int32 merge regression of another experimental approach did not recur. The inline candidate also retained zero estimation error across 20,500 additional allocator observations.

This dependency-side optimization is not part of the pushed DataFusion PR or a released hashbrown version. Wider consumer/compilation-cost validation and upstream hashbrown review are next. See the latest investigation update.

User-facing impact

Accurate retained-state accounting may trigger ResourcesExhausted earlier under tight memory budgets, while corrected double-counting avoids unnecessary overcharging.

[Nook001]

Quick update: I'm testing a further refinement to improve memory accounting accuracy for small primitive COUNT(DISTINCT) tables.

The candidate replaces std::collections::HashSet with DataFusion's existing datafusion_common::HashSet (hashbrown), retaining the explicit RandomState. This allows us to use allocation_size() directly, eliminating the remaining 16-byte estimation residual without hard-coded corrections or capacity-based layout assumptions.

Preliminary Windows validation is promising:

• 41 scenarios across 5 accumulator types (Float64, Int32, Int64, Decimal128, Decimal256).
• All 10,250 candidate retained-heap measurements showed zero estimation error.
• Targeted unit tests, aggregate.slt, formatting, and Clippy passed.

I'm proceeding with Linux allocation and performance validation before incorporating this refinement into the PR. I'll update the implementation and testing summary once the results are confirmed.

[Nook001]

Update: Linux validation and performance investigation

The hashbrown-based memory accounting change is now pushed in 46af5e0a7. Subsequent Rocky Linux validation confirmed zero retained-heap estimation error across all 10,250 candidate measurements.

However, controlled real-SQL benchmarks revealed a reproducible regression in nullable COUNT(DISTINCT) execution with the current hashbrown implementation. I have kept this PR in Draft while investigating it.

The investigation narrowed the regression to insert-path code generation and inlining. A separate, local hashbrown prototype changes one attribute on HashMap::insert from inline to inline(always) under the existing inline-more feature.

On Rocky Linux, compared with the matching unmodified hashbrown baseline:

• Nullable Int64 SQL: 18.83% faster.
• Decimal report SQL: 15.07% faster.
• Unique Int64 SQL: 7.58% faster.
• Across 40 microbenchmark combinations: 1.79% faster (equal-weight geometric mean).
• All 20,500 allocation observations retained zero estimation error, with unchanged allocation and capacity behavior.

The major Int32 merge regression observed with earlier experimental approaches did not reproduce. The main SQL improvements were also confirmed in independent interleaved runs.

I plan to validate its broader performance and compilation-cost impact before seeking upstream hashbrown review.

[alamb]

Thank you for the contribution. The change may well be correct, but I am closing this PR under our AI policy.

Maintainer review time is the scarcest resource in this project. This account has recently opened many AI-generated PRs across many repositories. That suggests the changes aren't coming from someone who uses DataFusion, and that no human will be around to discuss review feedback and maintain the code. Reviewing PRs like this costs us more time than it saves.

If you do use DataFusion and this fix matters to you, please pick the one change you care most about. Then open a focused PR with a short description in your own words, one you have reviewed and understand, and we'd be happy to help with review.

[Nook001]

@alamb
I'd like to clarify a few point.

First, I have never submitted AI-generated PRs to external open-source projects. Before this DataFusion PR, the only external projects I contributed PRs to were WinUI 3 and Electrobun, and I wrote both myself. All other PRs are for projects I own or develop with friends.

I don't understand why you claimed that I have submitted large numbers of AI-generated PRs across different projects. I would like to know what evidence you based this claim on.

Second, this PR is still a Draft. I didn't request a review. I am still working on the code, test data, and report, and investigating the performance regression. I don't understand how this could be considered an additional review burden when I haven't even asked anyone to review it.

Finally, I don't understand why you assume I don't use DataFusion, won't discuss review feedback, or won't maintain my contribution. I have been actively working on this PR, posting progress updates, running tests across different platforms, and exploring possible solutions.

I haven't even finished working on this PR yet, so I find it unreasonable to assume that I would abandon it or refuse to maintain it.

I'm not asking you to reopen this PR. I simply disagree with these unfair accusations and would like an explanation for them.