chore(storage/bloom/v1): update comments on bloom test (#14168)

pkg/storage/bloom/v1/bloom_tester.go

@@ -387,12 +387,12 @@ func (sm stringMatcherTest) Matches(bloom filter.Checker) bool {
 	)
 
 	if !bloom.Test(rawKey) {
-		// The structured metadata key wasn't indexed. We pass the bloom test
-		// since we can only filter data out if the key was indexed but the value
-		// wasn't.
+		// The structured metadata key wasn't indexed. However, sm.matcher might be
+		// checking against a label which *does* exist, so we can't safely filter
+		// out this chunk.
 		//
 		// TODO(rfratto): The negative test here is a bit confusing, and the key
-		// presence test should likely be done higher up.
+		// presence test should likely be done higher up within FuseQuerier.
 		return true
 	}
 
@@ -408,12 +408,12 @@ func (sm stringMatcherTest) MatchesWithPrefixBuf(bloom filter.Checker, buf []byt
 	)
 
 	if !bloom.Test(prefixedKey) {
-		// The structured metadata key wasn't indexed for a prefix. We pass the
-		// bloom test since we can only filter data out if the key was indexed but
-		// the value wasn't.
+		// The structured metadata key wasn't indexed for a prefix. However,
+		// sm.matcher might be checking against a label which *does* exist, so we
+		// can't safely filter out this chunk.
 		//
 		// TODO(rfratto): The negative test here is a bit confusing, and the key
-		// presence test should likely be done higher up.
+		// presence test should likely be done higher up within FuseQuerier.
 		return true
 	}
 

pkg/storage/bloom/v1/bloom_tester_test.go

@@ -168,6 +168,11 @@ func TestLabelMatchersToBloomTest(t *testing.T) {
 			query: `{app="fake"} | noexist="noexist"`,
 			match: true,
 		},
+		{
+			name:  "ignore non-indexed key with empty value",
+			query: `{app="fake"} | noexist=""`,
+			match: true,
+		},
 		{
 			name:  "ignore unsupported operator",
 			query: `{app="fake"} | trace_id=~".*noexist.*"`,