fred1268 commented on code in PR #23716:
URL: https://github.com/apache/datafusion/pull/23716#discussion_r3657989856
##########
datafusion/functions-aggregate/src/array_agg.rs:
##########
@@ -1546,8 +1682,10 @@ mod tests {
acc2.update_batch(&[string_list_data([vec!["e", "f", "g"]])])?;
acc1 = merge(acc1, acc2)?;
- // without compaction, the size is 16684
- assert_eq!(acc1.size(), 1684);
+ // The GroupValuesRows-based implementation uses a contiguous Rows
+ // buffer + HashTable instead of individual ScalarValue allocations,
+ // so the reported size differs from the previous implementation (was
1684).
Review Comment:
Sorry about this, I forgot to clean them up
##########
datafusion/functions-aggregate/src/array_agg.rs:
##########
@@ -876,12 +970,9 @@ impl Accumulator for DistinctArrayAggAccumulator {
assert_eq_or_internal_err!(states.len(), 1, "expects single state");
- // The DISTINCT state schema is `List<value>` — partial accumulators
- // ship the set of values they saw, not multiplicities. Re-ingesting
- // each element here makes the merged counts represent "partitions
- // that emitted this value," which is fine because `evaluate` only
- // reads keys. Refcount semantics for retract are only valid within
- // a single accumulator instance (window execution).
+ // The DISTINCT state is `List<value>`. Partial accumulators ship the
+ // set of live values, not multiplicities. Re-ingesting them here is
Review Comment:
Sorry about this, I forgot to clean them up
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
