lib/logstorage: optimize OR filters where one of these filters is *

Such filters can be optimized to `*`. This avoid executing other OR filters.
For example, `foo or * or bar` is optimized to `*`, while `foo` and `bar` filters aren't executed.

Such filters are frequently generated by Grafana, so this should improve query performance there.

Author: valyala

docs/victorialogs/CHANGELOG.md

@@ -45,6 +45,7 @@ according to [these docs](https://docs.victoriametrics.com/victorialogs/quicksta
 * FEATURE: [Journald data ingestion](https://docs.victoriametrics.com/victorialogs/data-ingestion/journald/): automatically add `level` log field according to [the `PRIORITY` field value](https://wiki.archlinux.org/title/Systemd/Journal#Priority_level). This enables [log level highlighting in Grafana](https://grafana.com/docs/grafana/latest/explore/logs-integration/#log-level). See [#8535](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/8535).
 * FEATURE: [Syslog data ingestion](https://docs.victoriametrics.com/victorialogs/data-ingestion/syslog/): automatically add `level` log field according to [the `severity` field value](https://en.wikipedia.org/wiki/Syslog#Severity_level). This enables [log level highlighting in Grafana](https://grafana.com/docs/grafana/latest/explore/logs-integration/#log-level).
 * FEATURE: [Journald data ingestion](https://docs.victoriametrics.com/victorialogs/data-ingestion/journald/): use `(_MACHINE_ID, _HOSTNAME, _SYSTEMD_UNIT)` fields as [log stream fields](https://docs.victoriametrics.com/victorialogs/keyconcepts/#stream-fields) by default. See [#9143](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/9143).
+* FEATURE: [LogsQL](https://docs.victoriametrics.com/victorialogs/logsql/): optimize `(any_filter or *)` filters to `*`. This avoids executing the `any_filter`. Such filters are frequently generated by Grafana.
 
 * BUGFIX: [query API](https://docs.victoriametrics.com/victorialogs/querying/#querying-logs): properly set storage node authorization in cluster mode when [Basic Auth](https://docs.victoriametrics.com/victorialogs/cluster/#security) is enabled. See [#9080](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/9080).
 * BUGFIX: [Journald data ingestion](https://docs.victoriametrics.com/victorialogs/data-ingestion/journald/): properly read log timestamp from `__REALTIME_TIMESTAMP` field according to [the docs](https://docs.victoriametrics.com/victorialogs/data-ingestion/journald/#time-field). See [#9144](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/9144). The bug has been introduced in [v1.22.0-victorialogs](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.22.0-victorialogs).

lib/logstorage/filter.go

@@ -19,30 +19,34 @@ type filter interface {
 	applyToBlockResult(br *blockResult, bm *bitmap)
 }
 
-// visitFilter sequentially calls visitFunc for filters inside f.
+// visitFilterRecursive recursively calls visitFunc for filters inside f.
 //
 // It stops calling visitFunc on the remaining filters as soon as visitFunc returns true.
 // It returns the result of the last visitFunc call.
-func visitFilter(f filter, visitFunc func(f filter) bool) bool {
+func visitFilterRecursive(f filter, visitFunc func(f filter) bool) bool {
+	return visitFilterInternal(f, visitFunc) || visitFunc(f)
+}
+
+func visitFilterInternal(f filter, visitFunc func(f filter) bool) bool {
 	switch t := f.(type) {
 	case *filterAnd:
-		return visitFilters(t.filters, visitFunc)
+		return visitFiltersRecursive(t.filters, visitFunc)
 	case *filterOr:
-		return visitFilters(t.filters, visitFunc)
+		return visitFiltersRecursive(t.filters, visitFunc)
 	case *filterNot:
-		return visitFilter(t.f, visitFunc)
+		return visitFilterRecursive(t.f, visitFunc)
 	default:
-		return visitFunc(f)
+		return false
 	}
 }
 
-// visitFilters calls visitFunc per each filter in filters.
+// visitFiltersRecursive recursively calls visitFunc per each filter in filters.
 //
 // It stops calling visitFunc on the remaining filters as soon as visitFunc returns true.
 // It returns the result of the last visitFunc call.
-func visitFilters(filters []filter, visitFunc func(f filter) bool) bool {
+func visitFiltersRecursive(filters []filter, visitFunc func(f filter) bool) bool {
 	for _, f := range filters {
-		if visitFilter(f, visitFunc) {
+		if visitFilterRecursive(f, visitFunc) {
 			return true
 		}
 	}
@@ -53,6 +57,11 @@ func visitFilters(filters []filter, visitFunc func(f filter) bool) bool {
 //
 // It doesn't copy other filters by returning them as is.
 func copyFilter(f filter, visitFunc func(f filter) bool, copyFunc func(f filter) (filter, error)) (filter, error) {
+	if !visitFilterRecursive(f, visitFunc) {
+		// Nothing to copy
+		return f, nil
+	}
+
 	f, err := copyFilterInternal(f, visitFunc, copyFunc)
 	if err != nil {
 		return nil, err
@@ -101,12 +110,6 @@ func copyFilterInternal(f filter, visitFunc func(f filter) bool, copyFunc func(f
 //
 // It doesn't copy other filters by returning them as is.
 func copyFilters(filters []filter, visitFunc func(f filter) bool, copyFunc func(f filter) (filter, error)) ([]filter, error) {
-	if !visitFilters(filters, visitFunc) {
-		// Nothing to copy
-		return filters, nil
-	}
-
-	// Copy filters.
 	filtersNew := make([]filter, len(filters))
 	for i, f := range filters {
 		fNew, err := copyFilter(f, visitFunc, copyFunc)

lib/logstorage/filter_or_test.go

@@ -80,6 +80,15 @@ func TestGetCommonTokensForOrFilters(t *testing.T) {
 		if err != nil {
 			t.Fatalf("unexpected error in ParseQuery: %s", err)
 		}
+
+		if _, ok := q.f.(*filterNoop); ok {
+			// The query is noop, so there are no common tokens
+			if len(tokensExpected) > 0 {
+				t.Fatalf("got no tokens, expecting tokens %q", tokensExpected)
+			}
+			return
+		}
+
 		fo, ok := q.f.(*filterOr)
 		if !ok {
 			t.Fatalf("unexpected filter type: %T; want *filterOr", q.f)
@@ -103,7 +112,7 @@ func TestGetCommonTokensForOrFilters(t *testing.T) {
 	// no common tokens
 	f(`foo OR bar`, nil)
 
-	// star filter matches non-empty value; it is skipped, since one of OR filters may contain an empty filter
+	// OR filters with star are reduced to filterNoop
 	f(`* OR foo`, nil)
 	f(`foo or *`, nil)
 	f(`* or *`, nil)

lib/logstorage/parser.go

@@ -666,7 +666,7 @@ func visitSubqueriesInFilter(f filter, visitFunc func(q *Query)) {
 		}
 		return false
 	}
-	_ = visitFilter(f, callback)
+	_ = visitFilterRecursive(f, callback)
 }
 
 func mergeFiltersStream(f filter) filter {
@@ -986,6 +986,28 @@ func removeStarFilters(f filter) filter {
 		logger.Panicf("BUG: unexpected error: %s", err)
 	}
 
+	// Replace filterOr with filterNoop if one of its sub-filters are filterNoop
+	visitFunc = func(f filter) bool {
+		fo, ok := f.(*filterOr)
+		if !ok {
+			return false
+		}
+		for _, f := range fo.filters {
+			if _, ok := f.(*filterNoop); ok {
+				return true
+			}
+		}
+		return false
+	}
+	copyFunc = func(_ filter) (filter, error) {
+		fn := &filterNoop{}
+		return fn, nil
+	}
+	f, err = copyFilter(f, visitFunc, copyFunc)
+	if err != nil {
+		logger.Panicf("BUG: unexpected error: %s", err)
+	}
+
 	// Drop filterNoop inside filterAnd
 	visitFunc = func(f filter) bool {
 		fa, ok := f.(*filterAnd)

lib/logstorage/parser_test.go

@@ -142,9 +142,31 @@ func TestQuery_AddTimeFilter(t *testing.T) {
 	f(`* | format if (x:contains_all(options(ignore_global_time_filter=true) y | keep x)) "foo"`, `_time:[2024-12-25T14:56:43Z,2025-01-13T12:45:34Z] * | format if (x:contains_all(options(ignore_global_time_filter=true) y | fields x)) foo`)
 }
 
+func TestParseQuery_OptimizeStarFilters(t *testing.T) {
+	f := func(s, resultExpected string) {
+		t.Helper()
+
+		q, err := ParseQuery(s)
+		if err != nil {
+			t.Fatalf("unexpected error: %s", err)
+		}
+		result := q.String()
+		if result != resultExpected {
+			t.Fatalf("unexpected result; got\n%s\nwant\n%s", result, resultExpected)
+		}
+	}
+
+	f(`*`, `*`)
+	f(`foo * bar`, `foo bar`)
+	f(`foo or * or bar`, `*`)
+	f(`foo and (bar or *)`, `foo`)
+	f(`foo or (* or (baz and (x and *))) x`, `foo or x`)
+}
+
 func TestParseQuery_OptimizeStreamFilters(t *testing.T) {
 	f := func(s, resultExpected string) {
 		t.Helper()
+
 		q, err := ParseQuery(s)
 		if err != nil {
 			t.Fatalf("unexpected error: %s", err)

lib/logstorage/storage_search.go

@@ -698,7 +698,7 @@ func hasFilterInWithQueryForFilter(f filter) bool {
 			return false
 		}
 	}
-	return visitFilter(f, visitFunc)
+	return visitFilterRecursive(f, visitFunc)
 }
 
 func hasFilterInWithQueryForPipes(pipes []pipe) bool {
@@ -1192,7 +1192,7 @@ func hasStreamFilters(f filter) bool {
 		_, ok := f.(*filterStream)
 		return ok
 	}
-	return visitFilter(f, visitFunc)
+	return visitFilterRecursive(f, visitFunc)
 }
 
 func initStreamFilters(tenantIDs []TenantID, idb *indexdb, f filter) filter {