diff --git a/docs/VictoriaLogs/CHANGELOG.md b/docs/VictoriaLogs/CHANGELOG.md index 1859237204..5cf6b55040 100644 --- a/docs/VictoriaLogs/CHANGELOG.md +++ b/docs/VictoriaLogs/CHANGELOG.md @@ -29,7 +29,7 @@ according to [these docs](https://docs.victoriametrics.com/victorialogs/quicksta * BUGFIX: properly handle Logstash requests for Elasticsearch configuration when using `outputs.elasticsearch` in Logstash pipelines. Previously, the requests could be rejected with `400 Bad Request` response. Updates [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4750). * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): fix `not found index.js` error when loading vmui in VictoriaLogs. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6764). Thanks to @yincongcyincong for the [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6770). -* BUGFIX: fix filtering when logical operators `AND` and `OR` are used in query expression. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6554) for details. Thanks to @yincongcyincong for the [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6556). +* BUGFIX: properly execute queries with `OR` [filters](https://docs.victoriametrics.com/victorialogs/logsql/#logical-filter) for distinct [log fields](https://docs.victoriametrics.com/victorialogs/keyconcepts/#data-model). For example, `field1:foo OR field2:bar`. Previously logs matching these filters may be skipped during querying. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6554) for details. Thanks to @yincongcyincong for the [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6556). ## [v0.28.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v0.28.0-victorialogs) diff --git a/lib/logstorage/filter_and.go b/lib/logstorage/filter_and.go index 1ad922c1ff..513d3e23bf 100644 --- a/lib/logstorage/filter_and.go +++ b/lib/logstorage/filter_and.go @@ -112,10 +112,6 @@ func (fa *filterAnd) getByFieldTokens() []fieldTokens { return fa.byFieldTokens } -// https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6554 -// and filter shouldn't return or filter which result in -// bloom filter execute error interception. -// detail see: https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6556#issuecomment-2323643507 func (fa *filterAnd) initByFieldTokens() { m := make(map[string]map[string]struct{}) var fieldNames []string @@ -157,6 +153,11 @@ func (fa *filterAnd) initByFieldTokens() { case *filterSequence: tokens := t.getTokens() mergeFieldTokens(t.fieldName, tokens) + case *filterOr: + bfts := t.getByFieldTokens() + for _, bft := range bfts { + mergeFieldTokens(bft.field, bft.tokens) + } } } diff --git a/lib/logstorage/filter_and_test.go b/lib/logstorage/filter_and_test.go index 080ed790bd..c0f9ddf695 100644 --- a/lib/logstorage/filter_and_test.go +++ b/lib/logstorage/filter_and_test.go @@ -25,7 +25,8 @@ func TestFilterAnd(t *testing.T) { }, } - //non-empty intersection + // non-empty intersection + // foo:a AND foo:abc* fa := &filterAnd{ filters: []filter{ &filterPhrase{ @@ -41,6 +42,7 @@ func TestFilterAnd(t *testing.T) { testFilterMatchForColumns(t, columns, fa, "foo", []int{2, 6}) // reverse non-empty intersection + // foo:abc* AND foo:a fa = &filterAnd{ filters: []filter{ &filterPrefix{ @@ -56,6 +58,7 @@ func TestFilterAnd(t *testing.T) { testFilterMatchForColumns(t, columns, fa, "foo", []int{2, 6}) // the first filter mismatch + // foo:bc* AND foo:a fa = &filterAnd{ filters: []filter{ &filterPrefix{ @@ -71,6 +74,7 @@ func TestFilterAnd(t *testing.T) { testFilterMatchForColumns(t, columns, fa, "foo", nil) // the last filter mismatch + // foo:abc AND foo:foo* fa = &filterAnd{ filters: []filter{ &filterPhrase{ @@ -86,6 +90,7 @@ func TestFilterAnd(t *testing.T) { testFilterMatchForColumns(t, columns, fa, "foo", nil) // empty intersection + // foo:foo AND foo:abc* fa = &filterAnd{ filters: []filter{ &filterPhrase{ @@ -101,6 +106,7 @@ func TestFilterAnd(t *testing.T) { testFilterMatchForColumns(t, columns, fa, "foo", nil) // reverse empty intersection + // foo:abc* AND foo:foo fa = &filterAnd{ filters: []filter{ &filterPrefix{ @@ -115,6 +121,88 @@ func TestFilterAnd(t *testing.T) { } testFilterMatchForColumns(t, columns, fa, "foo", nil) + // empty value + // foo:"" AND bar:"" + fa = &filterAnd{ + filters: []filter{ + &filterExact{ + fieldName: "foo", + value: "", + }, + &filterExact{ + fieldName: "bar", + value: "", + }, + }, + } + testFilterMatchForColumns(t, columns, fa, "foo", []int{5}) + + // non-existing field with empty value + // foo:foo* AND bar:"" + fa = &filterAnd{ + filters: []filter{ + &filterPrefix{ + fieldName: "foo", + prefix: "foo", + }, + &filterExact{ + fieldName: "bar", + value: "", + }, + }, + } + testFilterMatchForColumns(t, columns, fa, "foo", []int{0, 1, 3, 4, 6}) + + // reverse non-existing field with empty value + // bar:"" AND foo:foo* + fa = &filterAnd{ + filters: []filter{ + &filterExact{ + fieldName: "bar", + value: "", + }, + &filterPrefix{ + fieldName: "foo", + prefix: "foo", + }, + }, + } + testFilterMatchForColumns(t, columns, fa, "foo", []int{0, 1, 3, 4, 6}) + + // non-existing field with non-empty value + // foo:foo* AND bar:* + fa = &filterAnd{ + filters: []filter{ + &filterPrefix{ + fieldName: "foo", + prefix: "foo", + }, + &filterPrefix{ + fieldName: "bar", + prefix: "", + }, + }, + } + testFilterMatchForColumns(t, columns, fa, "foo", nil) + + // reverse non-existing field with non-empty value + // bar:* AND foo:foo* + fa = &filterAnd{ + filters: []filter{ + &filterPrefix{ + fieldName: "bar", + prefix: "", + }, + &filterPrefix{ + fieldName: "foo", + prefix: "foo", + }, + }, + } + testFilterMatchForColumns(t, columns, fa, "foo", nil) + + // https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6554 + // foo:"a foo"* AND (foo:="a foobar" OR boo:bbbbbbb) fa = &filterAnd{ filters: []filter{ &filterPrefix{ @@ -136,4 +224,151 @@ func TestFilterAnd(t *testing.T) { }, } testFilterMatchForColumns(t, columns, fa, "foo", []int{1}) + + // foo:"a foo"* AND (foo:"abcd foobar" OR foo:foobar) + fa = &filterAnd{ + filters: []filter{ + &filterPrefix{ + fieldName: "foo", + prefix: "a foo", + }, + &filterOr{ + filters: []filter{ + &filterPhrase{ + fieldName: "foo", + phrase: "abcd foobar", + }, + &filterPhrase{ + fieldName: "foo", + phrase: "foobar", + }, + }, + }, + }, + } + testFilterMatchForColumns(t, columns, fa, "foo", []int{1, 6}) + + // (foo:foo* OR bar:baz) AND (bar:x OR foo:a) + fa = &filterAnd{ + filters: []filter{ + &filterOr{ + filters: []filter{ + &filterPrefix{ + fieldName: "foo", + prefix: "foo", + }, + &filterPhrase{ + fieldName: "bar", + phrase: "baz", + }, + }, + }, + &filterOr{ + filters: []filter{ + &filterPhrase{ + fieldName: "bar", + phrase: "x", + }, + &filterPhrase{ + fieldName: "foo", + phrase: "a", + }, + }, + }, + }, + } + testFilterMatchForColumns(t, columns, fa, "foo", []int{0, 1, 3, 4, 6}) + + // (foo:foo* OR bar:baz) AND (bar:x OR foo:xyz) + fa = &filterAnd{ + filters: []filter{ + &filterOr{ + filters: []filter{ + &filterPrefix{ + fieldName: "foo", + prefix: "foo", + }, + &filterPhrase{ + fieldName: "bar", + phrase: "baz", + }, + }, + }, + &filterOr{ + filters: []filter{ + &filterPhrase{ + fieldName: "bar", + phrase: "x", + }, + &filterPhrase{ + fieldName: "foo", + phrase: "xyz", + }, + }, + }, + }, + } + testFilterMatchForColumns(t, columns, fa, "foo", nil) + + // (foo:foo* OR bar:baz) AND (bar:* OR foo:xyz) + fa = &filterAnd{ + filters: []filter{ + &filterOr{ + filters: []filter{ + &filterPrefix{ + fieldName: "foo", + prefix: "foo", + }, + &filterPhrase{ + fieldName: "bar", + phrase: "baz", + }, + }, + }, + &filterOr{ + filters: []filter{ + &filterPrefix{ + fieldName: "bar", + prefix: "", + }, + &filterPhrase{ + fieldName: "foo", + phrase: "xyz", + }, + }, + }, + }, + } + testFilterMatchForColumns(t, columns, fa, "foo", nil) + + // (foo:foo* OR bar:baz) AND (bar:"" OR foo:xyz) + fa = &filterAnd{ + filters: []filter{ + &filterOr{ + filters: []filter{ + &filterPrefix{ + fieldName: "foo", + prefix: "foo", + }, + &filterPhrase{ + fieldName: "bar", + phrase: "baz", + }, + }, + }, + &filterOr{ + filters: []filter{ + &filterExact{ + fieldName: "bar", + value: "", + }, + &filterPhrase{ + fieldName: "foo", + phrase: "xyz", + }, + }, + }, + }, + } + testFilterMatchForColumns(t, columns, fa, "foo", []int{0, 1, 3, 4, 6}) } diff --git a/lib/logstorage/filter_or.go b/lib/logstorage/filter_or.go index fa5aab0217..db281c1b65 100644 --- a/lib/logstorage/filter_or.go +++ b/lib/logstorage/filter_or.go @@ -171,13 +171,28 @@ func (fo *filterOr) initByFieldTokens() { var byFieldTokens []fieldTokens for _, fieldName := range fieldNames { - commonTokens := getCommonTokens(m[fieldName]) - if len(commonTokens) > 0 { - byFieldTokens = append(byFieldTokens, fieldTokens{ - field: fieldName, - tokens: commonTokens, - }) + tokenss := m[fieldName] + if len(tokenss) != len(fo.filters) { + // The filter for the given fieldName is missing in some OR filters, + // so it is impossible to extract common tokens from these filters. + // Give up extracting common tokens from the remaining filters, + // since they may not cover log entries matching fieldName filters. + // This fixes https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6554 + byFieldTokens = nil + break } + commonTokens := getCommonTokens(tokenss) + if len(commonTokens) == 0 { + // Give up extracting common tokens from the remaining filters, + // since they may not cover log entries matching fieldName filters. + // This fixes https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6554 + byFieldTokens = nil + break + } + byFieldTokens = append(byFieldTokens, fieldTokens{ + field: fieldName, + tokens: commonTokens, + }) } fo.byFieldTokens = byFieldTokens diff --git a/lib/logstorage/filter_or_test.go b/lib/logstorage/filter_or_test.go index 944e6cc07b..d5855de70a 100644 --- a/lib/logstorage/filter_or_test.go +++ b/lib/logstorage/filter_or_test.go @@ -26,6 +26,7 @@ func TestFilterOr(t *testing.T) { } // non-empty union + // foo:23 OR foo:abc fo := &filterOr{ filters: []filter{ &filterPhrase{ @@ -41,6 +42,7 @@ func TestFilterOr(t *testing.T) { testFilterMatchForColumns(t, columns, fo, "foo", []int{2, 6, 9}) // reverse non-empty union + // foo:abc OR foo:23 fo = &filterOr{ filters: []filter{ &filterPrefix{ @@ -56,6 +58,7 @@ func TestFilterOr(t *testing.T) { testFilterMatchForColumns(t, columns, fo, "foo", []int{2, 6, 9}) // first empty result, second non-empty result + // foo:xabc* OR foo:23 fo = &filterOr{ filters: []filter{ &filterPrefix{ @@ -71,6 +74,7 @@ func TestFilterOr(t *testing.T) { testFilterMatchForColumns(t, columns, fo, "foo", []int{9}) // first non-empty result, second empty result + // foo:23 OR foo:xabc* fo = &filterOr{ filters: []filter{ &filterPhrase{ @@ -86,6 +90,7 @@ func TestFilterOr(t *testing.T) { testFilterMatchForColumns(t, columns, fo, "foo", []int{9}) // first match all + // foo:a OR foo:23 fo = &filterOr{ filters: []filter{ &filterPhrase{ @@ -101,6 +106,7 @@ func TestFilterOr(t *testing.T) { testFilterMatchForColumns(t, columns, fo, "foo", []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}) // second match all + // foo:23 OR foo:a fo = &filterOr{ filters: []filter{ &filterPrefix{ @@ -116,6 +122,7 @@ func TestFilterOr(t *testing.T) { testFilterMatchForColumns(t, columns, fo, "foo", []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}) // both empty results + // foo:x23 OR foo:xabc fo = &filterOr{ filters: []filter{ &filterPhrase{ @@ -129,4 +136,191 @@ func TestFilterOr(t *testing.T) { }, } testFilterMatchForColumns(t, columns, fo, "foo", nil) + + // non-existing column (last) + // foo:23 OR bar:xabc* + fo = &filterOr{ + filters: []filter{ + &filterPhrase{ + fieldName: "foo", + phrase: "23", + }, + &filterPrefix{ + fieldName: "bar", + prefix: "xabc", + }, + }, + } + testFilterMatchForColumns(t, columns, fo, "foo", []int{9}) + + // non-existing column (first) + // bar:xabc* OR foo:23 + fo = &filterOr{ + filters: []filter{ + &filterPhrase{ + fieldName: "foo", + phrase: "23", + }, + &filterPrefix{ + fieldName: "bar", + prefix: "xabc", + }, + }, + } + testFilterMatchForColumns(t, columns, fo, "foo", []int{9}) + + // (foo:23 AND bar:"") OR (foo:foo AND bar:*) + fo = &filterOr{ + filters: []filter{ + &filterAnd{ + filters: []filter{ + &filterPhrase{ + fieldName: "foo", + phrase: "23", + }, + &filterExact{ + fieldName: "bar", + value: "", + }, + }, + }, + &filterAnd{ + filters: []filter{ + &filterPhrase{ + fieldName: "foo", + phrase: "foo", + }, + &filterPrefix{ + fieldName: "bar", + prefix: "", + }, + }, + }, + }, + } + testFilterMatchForColumns(t, columns, fo, "foo", []int{9}) + + // (foo:23 AND bar:"") OR (foo:foo AND bar:"") + fo = &filterOr{ + filters: []filter{ + &filterAnd{ + filters: []filter{ + &filterPhrase{ + fieldName: "foo", + phrase: "23", + }, + &filterExact{ + fieldName: "bar", + value: "", + }, + }, + }, + &filterAnd{ + filters: []filter{ + &filterPhrase{ + fieldName: "foo", + phrase: "foo", + }, + &filterExact{ + fieldName: "bar", + value: "", + }, + }, + }, + }, + } + testFilterMatchForColumns(t, columns, fo, "foo", []int{0, 9}) + + // (foo:23 AND bar:"") OR (foo:foo AND baz:"") + fo = &filterOr{ + filters: []filter{ + &filterAnd{ + filters: []filter{ + &filterPhrase{ + fieldName: "foo", + phrase: "23", + }, + &filterExact{ + fieldName: "bar", + value: "", + }, + }, + }, + &filterAnd{ + filters: []filter{ + &filterPhrase{ + fieldName: "foo", + phrase: "foo", + }, + &filterExact{ + fieldName: "baz", + value: "", + }, + }, + }, + }, + } + testFilterMatchForColumns(t, columns, fo, "foo", []int{0, 9}) + + // (foo:23 AND bar:abc) OR (foo:foo AND bar:"") + fo = &filterOr{ + filters: []filter{ + &filterAnd{ + filters: []filter{ + &filterPhrase{ + fieldName: "foo", + phrase: "23", + }, + &filterPhrase{ + fieldName: "bar", + phrase: "abc", + }, + }, + }, + &filterAnd{ + filters: []filter{ + &filterPhrase{ + fieldName: "foo", + phrase: "foo", + }, + &filterExact{ + fieldName: "bar", + value: "", + }, + }, + }, + }, + } + testFilterMatchForColumns(t, columns, fo, "foo", []int{0}) + + // (foo:23 AND bar:abc) OR (foo:foo AND bar:*) + fo = &filterOr{ + filters: []filter{ + &filterAnd{ + filters: []filter{ + &filterPhrase{ + fieldName: "foo", + phrase: "23", + }, + &filterPhrase{ + fieldName: "bar", + phrase: "abc", + }, + }, + }, + &filterAnd{ + filters: []filter{ + &filterPhrase{ + fieldName: "foo", + phrase: "foo", + }, + &filterPrefix{ + fieldName: "bar", + prefix: "", + }, + }, + }, + }, + } + testFilterMatchForColumns(t, columns, fo, "foo", nil) }