hanahmily commented on code in PR #932:
URL:
https://github.com/apache/skywalking-banyandb/pull/932#discussion_r2678624718
##########
pkg/query/logical/measure/measure_plan_distributed.go:
##########
@@ -536,9 +536,19 @@ func (s *pushedDownAggregatedIterator) Close() error {
}
// deduplicateAggregatedDataPoints removes duplicate aggregated results from
multiple replicas
-// by keeping only one data point per group. Since replicas hold identical
data, aggregates
-// for the same group are identical across replicas.
-func deduplicateAggregatedDataPoints(dataPoints []*measurev1.DataPoint,
groupByTagsRefs [][]*logical.TagRef) ([]*measurev1.DataPoint, error) {
+// and aggregates count results by summing values per group.
+func deduplicateAggregatedDataPoints(
+ dataPoints []*measurev1.DataPoint,
+ groupByTagsRefs [][]*logical.TagRef,
+ agg *measurev1.QueryRequest_Aggregation,
+) ([]*measurev1.DataPoint, error) {
+ if agg == nil {
+ return dataPoints, nil
+ }
+ isCount := agg.GetFunction() ==
modelv1.AggregationFunction_AGGREGATION_FUNCTION_COUNT
+ if isCount {
+ return aggregateCountDataPoints(dataPoints, groupByTagsRefs,
agg.GetFieldName())
Review Comment:
Why did you choose a different method to implement COUNT? In my opinion,
it's not much different from SUM.
From the implementation, it looks like the first version you used for SUM,
which you have already updated to the version we merged.
--
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]
