hanahmily commented on code in PR #849:
URL: 
https://github.com/apache/skywalking-banyandb/pull/849#discussion_r2551384392


##########
banyand/internal/encoding/tag_encoder_test.go:
##########
@@ -59,7 +59,7 @@ func TestEncodeDecodeTagValues_Int64_WithNilValues(t 
*testing.T) {
        for _, tt := range tests {
                t.Run(tt.name, func(t *testing.T) {
                        bb := &bytes.Buffer{}
-                       err := EncodeTagValues(bb, tt.values, 
pbv1.ValueTypeInt64)
+                       _, err := EncodeTagValues(bb, tt.values, 
pbv1.ValueTypeInt64)

Review Comment:
   Verify the encoding type is expected. 



##########
banyand/internal/sidx/block.go:
##########
@@ -350,22 +350,24 @@ func (b *block) mustWriteTag(tagName string, td *tagData, 
bm *blockMetadata, ww
                addUnique(td.values[i].value)
        }
 
-       bf := generateBloomFilter(len(uniqueValues))
-       for v := range uniqueValues {
-               bf.Add(convert.StringToBytes(v))
-       }
-
-       bb.Buf = encodeBloomFilter(bb.Buf[:0], bf)
-       tm.filterBlock.offset = tfw.bytesWritten
-       tm.filterBlock.size = uint64(len(bb.Buf))
-       tfw.MustWrite(bb.Buf)
-       releaseBloomFilter(bf)
-
-       // Compute min/max for int64 tags during unique value iteration
        if td.valueType == pbv1.ValueTypeInt64 && hasMinMax {
                tm.min = encoding.Int64ToBytes(nil, minVal)
                tm.max = encoding.Int64ToBytes(nil, maxVal)
        }
+       isDictionaryEncoded := encodeType == encoding.EncodeTypeDictionary
+       isArrayType := td.valueType == pbv1.ValueTypeStrArr || td.valueType == 
pbv1.ValueTypeInt64Arr
+       if !isDictionaryEncoded || isArrayType {

Review Comment:
   Why can the string array type not be dictionary encoding? 



-- 
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]

Reply via email to