eye-gu commented on code in PR #1110:
URL:
https://github.com/apache/skywalking-banyandb/pull/1110#discussion_r3176395163
##########
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:
This is the review comment for the previous PR of copilot:
> `setComparatorFromFieldType` panics on unsupported `FieldType`. Since
`TopN()` is part of the streaming pipeline setup, a panic here can crash the
process from a misconfiguration or schema evolution. Consider converting this
to an error path (e.g., `drainErr`) and make the message reflect actual
`databasev1.FieldType` values (the current text mentions
`ValueTypeInt64/Float64`).
```suggestion
// 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")
}
baseComparator = func(a, b interface{}) int {
// Treat all values as equal to avoid type assumptions
and panics.
return 0
}
```
link: https://github.com/apache/skywalking-banyandb/pull/1001
--
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]
