OmCheeLin commented on code in PR #1110:
URL:
https://github.com/apache/skywalking-banyandb/pull/1110#discussion_r3173842908
##########
pkg/flow/streaming/topn.go:
##########
@@ -194,11 +201,39 @@ func (t *topNAggregatorGroup) getOrCreateGroup(group
string) *topNAggregator {
t.aggregatorGroup[group] = &topNAggregator{
topNAggregatorGroup: t,
treeMap: treemap.NewWith(t.comparator),
- dict: make(map[uint64]int64),
+ dict: make(map[uint64]interface{}),
}
return t.aggregatorGroup[group]
}
+func (t *topNAggregatorGroup) setComparatorFromFieldType() {
+ var baseComparator utils.Comparator
+ switch t.fieldType {
+ case databasev1.FieldType_FIELD_TYPE_INT:
+ baseComparator = utils.Int64Comparator
+ case databasev1.FieldType_FIELD_TYPE_FLOAT:
+ baseComparator = utils.Float64Comparator
+ default:
+ // Unsupported field type: log and fall back to a no-op
comparator instead of panicking.
+ if t.l != nil {
+ t.l.Error().
+ Interface("fieldType", t.fieldType).
+ Msg("unsupported field type for TopN;
defaulting to no-op comparator")
+ }
+ // Treat all values as equal to avoid type assumptions and
panics.
+ baseComparator = func(_, _ interface{}) int {
+ return 0
+ }
Review Comment:
Could you explain why the error is being swallowed here? The previous code
already filters out `FieldType_FIELD_TYPE_UNSPECIFIED` and excludes
non-int/non-float types, unless we plan to support TopN for other `FieldType`
later on?
##########
test/cases/measure/data/testdata/topn_metric_data.json:
##########
Review Comment:
This file seems not to be referenced.
--
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]
