Copilot commented on code in PR #869:
URL:
https://github.com/apache/skywalking-banyandb/pull/869#discussion_r2569511580
##########
banyand/measure/write_standalone.go:
##########
@@ -316,6 +449,75 @@ func handleTagFamily(schema *databasev1.Measure, req
*measurev1.WriteRequest, lo
return tagFamilies, fields
}
+func handleIndexModeWithSpec(schema *databasev1.Measure, req
*measurev1.WriteRequest,
+ locator partition.IndexRuleLocator, spec *measurev1.DataPointSpec,
+) []index.Field {
+ specFamilyMap := make(map[string]int)
+ specTagMaps := make(map[string]map[string]int)
+ for i, specFamily := range spec.GetTagFamilySpec() {
+ specFamilyMap[specFamily.GetName()] = i
+ tagMap := make(map[string]int)
+ for j, tagName := range specFamily.GetTagNames() {
+ tagMap[tagName] = j
+ }
+ specTagMaps[specFamily.GetName()] = tagMap
+ }
+
+ var fields []index.Field
+ for i := range schema.GetTagFamilies() {
+ tagFamilySpec := schema.GetTagFamilies()[i]
+ tfr := locator.TagFamilyTRule[i]
+
+ var srcFamily *modelv1.TagFamilyForWrite
+ specIdx, ok := specFamilyMap[tagFamilySpec.Name]
+ if ok && specIdx < len(req.DataPoint.TagFamilies) {
+ srcFamily = req.DataPoint.TagFamilies[specIdx]
+ }
+
+ specTagMap := specTagMaps[tagFamilySpec.Name]
+ for j := range tagFamilySpec.Tags {
+ t := tagFamilySpec.Tags[j]
+
+ var tagValue *modelv1.TagValue
+ if srcFamily != nil && specTagMap != nil {
+ if srcTagIdx, ok := specTagMap[t.Name]; ok &&
srcTagIdx < len(srcFamily.Tags) {
+ tagValue = srcFamily.Tags[srcTagIdx]
+ }
+ }
+ if tagValue == nil {
+ tagValue = pbv1.NullTagValue
+ }
+
+ encodeTagValue := encodeTagValue(t.Name, t.Type,
tagValue)
+ r, toIndex := tfr[t.Name]
+ fieldKey := index.FieldKey{}
+ if toIndex {
+ fieldKey.IndexRuleID = r.GetMetadata().GetId()
+ fieldKey.Analyzer = r.Analyzer
+ } else {
+ fieldKey.TagName = t.Name
+ }
+ if encodeTagValue.value != nil {
+ f := index.NewBytesField(fieldKey,
encodeTagValue.value)
+ f.Store = true
+ f.Index = toIndex
+ f.NoSort = r.GetNoSort()
+ fields = append(fields, f)
+ } else {
+ for _, val := range encodeTagValue.valueArr {
+ f := index.NewBytesField(fieldKey, val)
+ f.Store = true
+ f.Index = toIndex
+ f.NoSort = r.GetNoSort()
Review Comment:
Potential nil pointer dereference: When `toIndex` is false (i.e., the tag is
not indexed), `r` will be nil from the map lookup, but `r.GetNoSort()` is
called unconditionally at lines 504 and 511. This will cause a panic.
The correct pattern should be:
```go
if toIndex {
f.NoSort = r.GetNoSort()
} else {
f.NoSort = false // or some default value
}
```
The same issue exists in the original `handleIndexMode` function at lines
557 and 564, but this PR introduces it to the new `handleIndexModeWithSpec`
function as well.
```suggestion
if toIndex {
f.NoSort = r.GetNoSort()
} else {
f.NoSort = false
}
fields = append(fields, f)
} else {
for _, val := range encodeTagValue.valueArr {
f := index.NewBytesField(fieldKey, val)
f.Store = true
f.Index = toIndex
if toIndex {
f.NoSort = r.GetNoSort()
} else {
f.NoSort = false
}
f.NoSort = r.GetNoSort()
} else {
f.NoSort = false
}
f.NoSort = r.GetNoSort()
} else {
f.NoSort = false
}
```
--
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]
