From 02a922b53ff48b24b05814de88bdbde7eca4cc0f Mon Sep 17 00:00:00 2001
From: Aliaksandr Valialkin <valyala@victoriametrics.com>
Date: Thu, 24 Feb 2022 12:10:44 +0200
Subject: [PATCH] lib/storage: properly handle series selector matching
 multiple metric names plus a negative filter

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2238

This is a follow-up for 00cbb099b6444c2e285ef32ffce7b794186bc0be
---
 docs/CHANGELOG.md               |  1 +
 lib/storage/tag_filters.go      |  7 +++++--
 lib/storage/tag_filters_test.go | 18 ++----------------
 3 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index a0711ebf93..34f45b417b 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -33,6 +33,7 @@ This rule is equivalent to less clear traditional one:
 
 * FEATURE: reduce memory usage for various caches under [high churn rate](https://docs.victoriametrics.com/FAQ.html#what-is-high-churn-rate).
 
+* BUGFIX: properly handle [series selector](https://prometheus.io/docs/prometheus/latest/querying/basics/#time-series-selectors) containing a filter for multiple metric names plus a negative filter. For example, `{__name__=~"foo|bar",job!="baz"}` . Previously VictoriaMetrics could return series with `foo` or `bar` names and with `job="baz"`. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2238).
 * BUGFIX: [vmgateway](https://docs.victoriametrics.com/vmgateway.html): properly parse JWT tokens if they are encoded with [URL-safe base64 encoding](https://datatracker.ietf.org/doc/html/rfc4648#section-5).
 
 
diff --git a/lib/storage/tag_filters.go b/lib/storage/tag_filters.go
index 8428dc5220..90a03077ec 100644
--- a/lib/storage/tag_filters.go
+++ b/lib/storage/tag_filters.go
@@ -50,7 +50,10 @@ func convertToCompositeTagFilters(tfs *TagFilters) []*TagFilters {
 			hasPositiveFilter = true
 		}
 	}
-	if len(names) == 0 {
+	// If tfs have no filters on __name__ or have no non-negative filters,
+	// then it is impossible to construct composite tag filter.
+	// See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2238
+	if len(names) == 0 || !hasPositiveFilter {
 		atomic.AddUint64(&compositeFilterMissingConversions, 1)
 		return []*TagFilters{tfs}
 	}
@@ -62,7 +65,7 @@ func convertToCompositeTagFilters(tfs *TagFilters) []*TagFilters {
 		tfsNew := make([]tagFilter, 0, len(tfs.tfs))
 		for _, tf := range tfs.tfs {
 			if len(tf.key) == 0 {
-				if !hasPositiveFilter || tf.isNegative {
+				if tf.isNegative {
 					// Negative filters on metric name cannot be used for building composite filter, so leave them as is.
 					tfsNew = append(tfsNew, tf)
 					continue
diff --git a/lib/storage/tag_filters_test.go b/lib/storage/tag_filters_test.go
index dc128e2c70..88a4765e5e 100644
--- a/lib/storage/tag_filters_test.go
+++ b/lib/storage/tag_filters_test.go
@@ -193,7 +193,7 @@ func TestConvertToCompositeTagFilters(t *testing.T) {
 				IsRegexp:   false,
 			},
 			{
-				Key:        []byte("\xfe\x03barfoo"),
+				Key:        []byte("foo"),
 				Value:      []byte("abc"),
 				IsNegative: true,
 				IsRegexp:   false,
@@ -596,21 +596,7 @@ func TestConvertToCompositeTagFilters(t *testing.T) {
 				IsRegexp:   true,
 			},
 			{
-				Key:        []byte("\xfe\x03barfoo"),
-				Value:      []byte("abc"),
-				IsNegative: true,
-				IsRegexp:   false,
-			},
-		},
-		{
-			{
-				Key:        nil,
-				Value:      []byte("bar|foo"),
-				IsNegative: false,
-				IsRegexp:   true,
-			},
-			{
-				Key:        []byte("\xfe\x03foofoo"),
+				Key:        []byte("foo"),
 				Value:      []byte("abc"),
 				IsNegative: true,
 				IsRegexp:   false,