arunkumarucet commented on PR #19056: URL: https://github.com/apache/pinot/pull/19056#issuecomment-5066374415
Regarding the mixed `NonScanBasedAggregationOperator` / `AggregationOperator` concern: addressed in 975e25c. The problem was that block merge order is nondeterministic, so when segments mix the two paths (e.g. a filter that matches all docs in only some segments), a scan-side `LongOpenHashSet` could end up as the accumulator and absorb the sorted sets element-by-element, silently losing the optimization. Added `SortedLongDistinctSet.union(set1, set2)`, which makes the sorted set the absorbing side regardless of which operand it arrives on, and used it at every merge site that can see mixed types (`BaseDistinctAggregateAggregationFunction.merge` plus the SmartHLL/SmartHLLPlus/SmartULL set merges). Absorbing a hash set costs one sort+dedupe of that segment's values, which is comparable to what the hash merge did, and merge cost is now deterministic regardless of block arrival order. Added tests covering both operand orders at the helper level and through `DistinctCountAggregationFunction.merge`. -- 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]
