Copilot commented on code in PR #936:
URL:
https://github.com/apache/skywalking-banyandb/pull/936#discussion_r2685996265
##########
banyand/measure/topn.go:
##########
@@ -784,12 +778,14 @@ type TopNValue struct {
firstValue int64
}
-func (t *TopNValue) setMetadata(valueName string, entityTagNames []string) {
+// SetMetadata set valueName and entityTagNames.
Review Comment:
The comment should use proper grammar. It should be "Sets valueName and
entityTagNames" instead of "set valueName and entityTagNames".
```suggestion
// SetMetadata sets valueName and entityTagNames.
```
##########
banyand/measure/topn.go:
##########
@@ -784,12 +778,14 @@ type TopNValue struct {
firstValue int64
}
-func (t *TopNValue) setMetadata(valueName string, entityTagNames []string) {
+// SetMetadata set valueName and entityTagNames.
+func (t *TopNValue) SetMetadata(valueName string, entityTagNames []string) {
t.valueName = valueName
t.entityTagNames = entityTagNames
}
-func (t *TopNValue) addValue(value int64, entityValues []*modelv1.TagValue) {
+// AddValue add value and entityValues.
Review Comment:
The comment should use proper grammar. It should be "Adds value and
entityValues" instead of "add value and entityValues".
```suggestion
// AddValue adds value and entityValues.
```
##########
banyand/measure/topn.go:
##########
@@ -842,7 +838,8 @@ func (t *TopNValue) resizeEntities(size, entitySize int)
[][]*modelv1.TagValue {
return t.entities
}
-func (t *TopNValue) marshal(dst []byte) ([]byte, error) {
+// Marshal marshal the topNValue to the dst.
Review Comment:
The comment should use proper grammar. It should be "Marshals the topNValue
to the dst" instead of "marshal the topNValue to the dst".
```suggestion
// Marshal marshals the topNValue to the dst.
```
##########
banyand/measure/block.go:
##########
@@ -704,8 +706,80 @@ func (bc *blockCursor) replace(r *model.MeasureResult,
storedIndexValue map[comm
}
}
}
- for i, c := range bc.fields.columns {
- r.Fields[i].Values[len(r.Fields[i].Values)-1] =
mustDecodeFieldValue(c.valueType, c.values[bc.idx])
+
+ if topNPostAggregator != nil {
+ topNValue := GenerateTopNValue()
+ defer ReleaseTopNValue(topNValue)
+ decoder := GenerateTopNValuesDecoder()
+ defer ReleaseTopNValuesDecoder(decoder)
+
+ uTimestamps := uint64(bc.timestamps[bc.idx])
+
+ for i, c := range bc.fields.columns {
+ srcFieldValue :=
r.Fields[i].Values[len(r.Fields[i].Values)-1]
+ destFieldValue := mustDecodeFieldValue(c.valueType,
c.values[bc.idx])
+
+ topNValue.Reset()
+
+ if err :=
topNValue.Unmarshal(srcFieldValue.GetBinaryData(), decoder); err != nil {
+ log.Error().Err(err).Msg("failed to unmarshal
topN value, skip current batch")
+ continue
+ }
+
+ valueName := topNValue.valueName
+ entityTagNames := topNValue.entityTagNames
+
+ for j, entityList := range topNValue.entities {
+ entityValues := make(pbv1.EntityValues, 0,
len(topNValue.entityValues))
+ for _, e := range entityList {
+ entityValues = append(entityValues, e)
+ }
+ topNPostAggregator.Put(entityValues,
topNValue.values[j], uTimestamps, bc.versions[bc.idx])
+ }
+
+ topNValue.Reset()
+ if err :=
topNValue.Unmarshal(destFieldValue.GetBinaryData(), decoder); err != nil {
+ log.Error().Err(err).Msg("failed to unmarshal
topN value, skip current batch")
+ continue
+ }
+
+ for j, entityList := range topNValue.entities {
+ entityValues := make(pbv1.EntityValues, 0,
len(topNValue.entityValues))
Review Comment:
The capacity initialization for entityValues is incorrect. It should use
len(entityList) instead of len(topNValue.entityValues). The field
topNValue.entityValues is a [][]byte used for serialization, not the actual
entity list. This will cause an incorrectly sized slice allocation.
##########
banyand/measure/block.go:
##########
@@ -704,8 +706,80 @@ func (bc *blockCursor) replace(r *model.MeasureResult,
storedIndexValue map[comm
}
}
}
- for i, c := range bc.fields.columns {
- r.Fields[i].Values[len(r.Fields[i].Values)-1] =
mustDecodeFieldValue(c.valueType, c.values[bc.idx])
+
+ if topNPostAggregator != nil {
+ topNValue := GenerateTopNValue()
+ defer ReleaseTopNValue(topNValue)
+ decoder := GenerateTopNValuesDecoder()
+ defer ReleaseTopNValuesDecoder(decoder)
+
+ uTimestamps := uint64(bc.timestamps[bc.idx])
+
+ for i, c := range bc.fields.columns {
+ srcFieldValue :=
r.Fields[i].Values[len(r.Fields[i].Values)-1]
+ destFieldValue := mustDecodeFieldValue(c.valueType,
c.values[bc.idx])
+
+ topNValue.Reset()
+
+ if err :=
topNValue.Unmarshal(srcFieldValue.GetBinaryData(), decoder); err != nil {
+ log.Error().Err(err).Msg("failed to unmarshal
topN value, skip current batch")
+ continue
+ }
+
+ valueName := topNValue.valueName
+ entityTagNames := topNValue.entityTagNames
+
+ for j, entityList := range topNValue.entities {
+ entityValues := make(pbv1.EntityValues, 0,
len(topNValue.entityValues))
Review Comment:
The capacity initialization for entityValues is incorrect. It should use
len(entityList) instead of len(topNValue.entityValues). The field
topNValue.entityValues is a [][]byte used for serialization, not the actual
entity list. This will cause an incorrectly sized slice allocation.
--
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]
