james-willis opened a new pull request, #1227: URL: https://github.com/apache/sedona-db/pull/1227
Follow-up to #1225: after fixing `ST_Collect_Agg`'s memory accounting, we audited every other `Accumulator`/`GroupsAccumulator` `size()` implementation in the repo (enumerated via the trait-required `fn size(&self) -> usize`, cross-checked with a repo-wide sweep for per-group set/collection state). Three more accounting gaps turned up, each small; this PR fixes all three as separate commits. No behavior changes — only what gets reported to the memory pool. ## 1. `ItemCrsAccumulator` (sedona-expr) `size()` returned `inner.size() + size_of::<ItemCrsAccumulator>()`, omitting the per-group `crs: Option<String>` heap. Since this wrapper multiplies across every aggregate over item-CRS arguments, a high-cardinality `GROUP BY` leaked one uncounted String allocation per group. Fix adds the string's capacity. ## 2. `ST_Union_Agg` / `ST_Intersection_Agg` (sedona-geo) `size()` counted only coordinates, and only when the held geometry matched the `Geometry::MultiPolygon` variant — any other variant contributed **zero** — and container overhead (the `MultiPolygon`'s polygon buffer, per-ring `LineString` headers, interior-ring buffers) was never counted. Both accumulators now share a variant-exhaustive `geometry_heap_size()` walk (new crate-private `geometry_mem` module, with tests). The MultiPolygon-only internal invariant these accumulators maintain still holds; the total walk just makes `size()` robust instead of silently reporting zero for unmatched shapes. ## 3. `BoundsGroupsAccumulator2D` (sedona-functions) The groups accumulator summed per-bounder `mem_used()` over the *live* bounders, missing the `bounders: Vec<T>` doubling slack (capacity beyond len). Smallest of the three — added the slack term. ## Not in scope `ST_ConvexHull_Agg` and `ST_Analyze_Agg` were audited and are already honest (capacity-based counting and the u32 bitset respectively). The larger `ST_Collect_Agg` follow-ups discussed in #1225 (native `GroupsAccumulator`, DataFusion-side adapter accounting) remain separate work. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
