Repository navigation
FFI: FFI_GroupsAccumulator missing size #22335
Description
Gap
FFI_GroupsAccumulator in datafusion/ffi/src/udaf/groups_accumulator.rs does not plumb GroupsAccumulator::size. The consumer side cannot ask the producer how much memory the accumulator currently holds.
Missing method
size
Why it matters
Severity: minor. Memory accounting on the consumer side (spill / memory-pool tracking) reports whatever the trait default returns rather than the producer's real footprint. Wrong answers may cause unnecessary spills or, worse, OOMs that go uncaught by the pool.
Implementation notes
- Plumb as
unsafe extern \"C\" fn(this: &Self) -> usize; wrapper body callsinner.size(). - Layout change →
api changelabel, targetmainonly, no back-port tobranch-<major>. - Add a unit test (forced-foreign via
mock_foreign_marker_id) asserting the consumer-sidesize()matches the producer's response. Integration test underdatafusion/ffi/tests/if the producer side is exercised cross-library.
Generated from datafusion-ffi skill audit. See .ai/skills/datafusion-ffi/SKILL.md §"Method coverage" and §"Known gaps to close" (originated in PR #22327). If a PR addressing this finds the omission to be a false positive, please also propose an update to the datafusion-ffi skill so future audits do not re-flag it.
Activity
kumarUjjawal commentedon May 21, 2026
Claude:
sizeis already plumbed indatafusion/ffi/src/udaf/groups_accumulator.rs, has been since theoriginal PR #14775 (Dec 2024):
groups_accumulator.rs:56(https://github.com/apache/datafusion/blob/main/datafusion/ffi/src/udaf/gro
ups_accumulator.rs#L170-L175) — size_fn_wrapper calls inner.size().
(https://github.com/apache/datafusion/blob/main/datafusion/ffi/src/udaf/gro
ups_accumulator.rs#L264).
(https://github.com/apache/datafusion/blob/main/datafusion/ffi/src/udaf/gro
ups_accumulator.rs#L347-L349).
groups_accumulator.rs:559-588 (https://github.com/apache/datafusion/blob/ma
in/datafusion/ffi/src/udaf/groups_accumulator.rs#L559-L588)
(test_ffi_groups_accumulator_local_bypass_inner).
timsaucer commentedon May 21, 2026
I've found that the reason this was falsely identified was that the skill would use it's memory/inference instead of validating against the file contents. Thanks for looking into it!
docs(ffi-skill): require dual-grep audit to prevent false-positive ga…