JingsongLi commented on code in PR #10607:
URL: https://github.com/apache/paimon/pull/10607#discussion_r4294719990
##########
paimon-core/src/main/java/org/apache/paimon/mergetree/compact/aggregate/FieldCollectAgg.java:
##########
@@ -130,16 +125,21 @@ private Object agg(Object accumulator, Object inputField,
boolean accumulatorHas
return inputArray;
}
- boolean removeDuplicate = distinct && equaliser != null;
Collection<Object> collection =
distinct && equaliser == null ? new LinkedHashSet<>() : new
ArrayList<>();
if (countLimit >= 0) {
- collect(collection, accumulatorArray, removeDuplicate &&
accumulatorHasDuplicate);
- collect(collection, inputArray, removeDuplicate);
+ collect(
+ collection,
+ accumulatorArray,
+ checkDuplicateUntil(accumulatorHasDuplicate, collection));
+ collect(collection, inputArray,
checkDuplicateUntil(inputHasDuplicate, collection));
return new GenericArray(collection.toArray());
} else {
- collect(collection, inputArray, removeDuplicate);
- collect(collection, accumulatorArray, removeDuplicate);
+ collect(collection, inputArray,
checkDuplicateUntil(inputHasDuplicate, collection));
+ collect(
+ collection,
+ accumulatorArray,
+ checkDuplicateUntil(accumulatorHasDuplicate, collection));
Review Comment:
[P2] Handle accumulators initialized from DELETE records before skipping
internal deduplication
The accumulator is not always already distinct: with
`aggregation.remove-record-on-delete=true`, `AggregateMergeFunction.add`
handles DELETE by copying its raw fields through `initRow`, without invoking
the field aggregator. The next INSERT reaches this new limited-comparison path.
I reproduced `ARRAY<ROW<INT>>`, `distinct=true`, `count-limit=-3`, DELETE
`[(1),(1)]`, then INSERT `[(2)]`: the production merge function returns
`[(1),(1),(2)]` on this head versus `[(1),(2)]` with the base implementation.
Comparing accumulator elements only against the earlier input prefix leaves its
internal duplicates in the result. Please establish/normalize the
distinct-state invariant for these callers (or retain full deduplication when
it cannot be guaranteed), and cover DELETE followed by INSERT in a
merge-function regression test.
--
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]
