From 0f7d226b9b4e71aceafd09b8105764d0c9697ab4 Mon Sep 17 00:00:00 2001 From: Nityananda Gohain Date: Wed, 21 May 2025 10:22:42 +0530 Subject: [PATCH] Fix: exists clause in logs QB (#7987) * fix: exists in logs QB * fix: exists in logs json qb --- pkg/query-service/app/logs/v4/json_filter.go | 13 ++-- .../app/logs/v4/json_filter_test.go | 40 +++++++++++++ .../app/logs/v4/query_builder.go | 16 ++++- .../app/logs/v4/query_builder_test.go | 59 +++++++++++++++++++ .../app/resource/resource_query_builder.go | 20 ++++--- .../resource/resource_query_builder_test.go | 16 +++++ 6 files changed, 150 insertions(+), 14 deletions(-) diff --git a/pkg/query-service/app/logs/v4/json_filter.go b/pkg/query-service/app/logs/v4/json_filter.go index f7351cda72..7dc9a28acc 100644 --- a/pkg/query-service/app/logs/v4/json_filter.go +++ b/pkg/query-service/app/logs/v4/json_filter.go @@ -80,10 +80,15 @@ func GetJSONFilter(item v3.FilterItem) (string, error) { filters := []string{} - pathFilter := logsV3.GetPathIndexFilter(item.Key.Key) - if pathFilter != "" { - filters = append(filters, pathFilter) + // don't add path filter for !=, not exists, not in, not like, not contains + // it's not compatible with the index filter + if _, ok := skipExistsFilter[op]; !ok { + pathFilter := logsV3.GetPathIndexFilter(item.Key.Key) + if pathFilter != "" { + filters = append(filters, pathFilter) + } } + if op == v3.FilterOperatorContains || op == v3.FilterOperatorEqual || op == v3.FilterOperatorHas { @@ -94,7 +99,7 @@ func GetJSONFilter(item v3.FilterItem) (string, error) { } // add exists check for non array items as default values of int/float/bool will corrupt the results - if !isArray && !(item.Operator == v3.FilterOperatorExists || item.Operator == v3.FilterOperatorNotExists) { + if _, ok := skipExistsFilter[op]; !ok && !isArray && op != v3.FilterOperatorExists { existsFilter := fmt.Sprintf("JSON_EXISTS(body, '$.%s')", logsV3.GetPath(strings.Split(item.Key.Key, ".")[1:])) filter = fmt.Sprintf("%s AND %s", existsFilter, filter) } diff --git a/pkg/query-service/app/logs/v4/json_filter_test.go b/pkg/query-service/app/logs/v4/json_filter_test.go index 2456aa4c45..63aa1749bf 100644 --- a/pkg/query-service/app/logs/v4/json_filter_test.go +++ b/pkg/query-service/app/logs/v4/json_filter_test.go @@ -248,6 +248,46 @@ var testGetJSONFilterData = []struct { }, Filter: "lower(body) like lower('%value%') AND JSON_EXISTS(body, '$.\"value\"') AND JSON_VALUE(body, '$.\"value\"') IN ['hello','11']", }, + + // test !=, not exists, not in, not like, not contains + { + Name: "neq operator", + FilterItem: v3.FilterItem{ + Key: v3.AttributeKey{ + Key: "body.message", + DataType: "string", + IsJSON: true, + }, + Operator: "!=", + Value: "hello", + }, + Filter: "JSON_VALUE(body, '$.\"message\"') != 'hello'", + }, + { + Name: "not exists", + FilterItem: v3.FilterItem{ + Key: v3.AttributeKey{ + Key: "body.message", + DataType: "string", + IsJSON: true, + }, + Operator: "nexists", + }, + Filter: "NOT JSON_EXISTS(body, '$.\"message\"')", + }, + { + Name: "not in", + FilterItem: v3.FilterItem{ + Key: v3.AttributeKey{ + Key: "body.message", + DataType: "string", + IsJSON: true, + }, + Operator: "nin", + Value: []interface{}{"hello", "world"}, + }, + Filter: "JSON_VALUE(body, '$.\"message\"') NOT IN ['hello','world']", + }, } func TestGetJSONFilter(t *testing.T) { diff --git a/pkg/query-service/app/logs/v4/query_builder.go b/pkg/query-service/app/logs/v4/query_builder.go index efaf558069..e97ec9a75d 100644 --- a/pkg/query-service/app/logs/v4/query_builder.go +++ b/pkg/query-service/app/logs/v4/query_builder.go @@ -30,6 +30,15 @@ var logOperators = map[v3.FilterOperator]string{ v3.FilterOperatorNotExists: "not mapContains(%s_%s, '%s')", } +var skipExistsFilter = map[v3.FilterOperator]struct{}{ + v3.FilterOperatorNotEqual: {}, + v3.FilterOperatorNotLike: {}, + v3.FilterOperatorNotContains: {}, + v3.FilterOperatorNotRegex: {}, + v3.FilterOperatorNotIn: {}, + v3.FilterOperatorNotExists: {}, +} + const ( BODY = "body" DISTRIBUTED_LOGS_V2 = "distributed_logs_v2" @@ -204,11 +213,14 @@ func buildLogsTimeSeriesFilterQuery(fs *v3.FilterSet, groupBy []v3.AttributeKey, } conditions = append(conditions, filter) + op := v3.FilterOperator(strings.ToLower(string(item.Operator))) + // add extra condition for map contains // by default clickhouse is not able to utilize indexes for keys with all operators. // mapContains forces the use of index. - op := v3.FilterOperator(strings.ToLower(string(item.Operator))) - if item.Key.IsColumn == false && op != v3.FilterOperatorExists && op != v3.FilterOperatorNotExists { + // for mat column it's is not required as it will already use the dedicated index. + // skip the exists filter for operators such as !=, not like, not contains, not regex, not in + if _, ok := skipExistsFilter[op]; !ok && item.Key.IsColumn == false && item.Operator != v3.FilterOperatorExists { conditions = append(conditions, getExistsNexistsFilter(v3.FilterOperatorExists, item)) } } diff --git a/pkg/query-service/app/logs/v4/query_builder_test.go b/pkg/query-service/app/logs/v4/query_builder_test.go index b35abe68ae..75165bab9b 100644 --- a/pkg/query-service/app/logs/v4/query_builder_test.go +++ b/pkg/query-service/app/logs/v4/query_builder_test.go @@ -433,6 +433,65 @@ func Test_buildLogsTimeSeriesFilterQuery(t *testing.T) { want: "attributes_string['service.name'] = 'test' AND mapContains(attributes_string, 'service.name') " + "AND mapContains(attributes_string, 'user_name') AND `attribute_string_method_exists`=true AND mapContains(attributes_string, 'test')", }, + { + name: "Shouldn't add exists filter for operators !=, not like, not contains, not regex, not in", + args: args{ + fs: &v3.FilterSet{ + Items: []v3.FilterItem{ + { + Key: v3.AttributeKey{ + Key: "service.name", + DataType: v3.AttributeKeyDataTypeString, + Type: v3.AttributeKeyTypeTag, + }, + Operator: v3.FilterOperatorNotEqual, + Value: "test", + }, + { + Key: v3.AttributeKey{ + Key: "service.name", + DataType: v3.AttributeKeyDataTypeString, + Type: v3.AttributeKeyTypeTag, + }, + Operator: v3.FilterOperatorNotLike, + Value: "test%", + }, + { + Key: v3.AttributeKey{ + Key: "service.name", + DataType: v3.AttributeKeyDataTypeString, + Type: v3.AttributeKeyTypeTag, + }, + Operator: v3.FilterOperatorNotContains, + Value: "test", + }, + { + Key: v3.AttributeKey{ + Key: "service.name", + DataType: v3.AttributeKeyDataTypeString, + Type: v3.AttributeKeyTypeTag, + }, + Operator: v3.FilterOperatorNotRegex, + Value: "^test", + }, + { + Key: v3.AttributeKey{ + Key: "service.name", + DataType: v3.AttributeKeyDataTypeString, + Type: v3.AttributeKeyTypeTag, + }, + Operator: v3.FilterOperatorNotIn, + Value: []string{"test"}, + }, + }, + }, + }, + want: "attributes_string['service.name'] != 'test' " + + "AND attributes_string['service.name'] NOT ILIKE 'test%' " + + "AND attributes_string['service.name'] NOT ILIKE '%test%' " + + "AND NOT match(attributes_string['service.name'], '^test') " + + "AND attributes_string['service.name'] NOT IN ['test']", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/query-service/app/resource/resource_query_builder.go b/pkg/query-service/app/resource/resource_query_builder.go index 99f6864198..a218886183 100644 --- a/pkg/query-service/app/resource/resource_query_builder.go +++ b/pkg/query-service/app/resource/resource_query_builder.go @@ -116,18 +116,22 @@ func buildResourceIndexFilter(key string, op v3.FilterOperator, value interface{ // add index filters switch op { - case v3.FilterOperatorContains: - return fmt.Sprintf("lower(labels) like '%%%s%%%s%%'", key, fmtValEscapedForContainsLower) - case v3.FilterOperatorNotContains: - return fmt.Sprintf("lower(labels) not like '%%%s%%%s%%'", key, fmtValEscapedForContainsLower) - case v3.FilterOperatorLike: - return fmt.Sprintf("lower(labels) like '%%%s%%%s%%'", key, fmtValEscapedLower) - case v3.FilterOperatorNotLike: - return fmt.Sprintf("lower(labels) not like '%%%s%%%s%%'", key, fmtValEscapedLower) case v3.FilterOperatorEqual: return fmt.Sprintf("labels like '%%%s%%%s%%'", key, fmtValEscapedForContains) case v3.FilterOperatorNotEqual: return fmt.Sprintf("labels not like '%%%s%%%s%%'", key, fmtValEscapedForContains) + case v3.FilterOperatorLike: + return fmt.Sprintf("lower(labels) like '%%%s%%%s%%'", key, fmtValEscapedLower) + case v3.FilterOperatorNotLike: + return fmt.Sprintf("lower(labels) not like '%%%s%%%s%%'", key, fmtValEscapedLower) + case v3.FilterOperatorContains: + return fmt.Sprintf("lower(labels) like '%%%s%%%s%%'", key, fmtValEscapedForContainsLower) + case v3.FilterOperatorNotContains: + return fmt.Sprintf("lower(labels) not like '%%%s%%%s%%'", key, fmtValEscapedForContainsLower) + case v3.FilterOperatorExists: + return fmt.Sprintf("lower(labels) like '%%%s%%'", key) + case v3.FilterOperatorNotExists: + return fmt.Sprintf("lower(labels) not like '%%%s%%'", key) case v3.FilterOperatorRegex, v3.FilterOperatorNotRegex: // don't try to do anything for regex. return "" diff --git a/pkg/query-service/app/resource/resource_query_builder_test.go b/pkg/query-service/app/resource/resource_query_builder_test.go index 4afee8a92e..7e867ddc14 100644 --- a/pkg/query-service/app/resource/resource_query_builder_test.go +++ b/pkg/query-service/app/resource/resource_query_builder_test.go @@ -235,6 +235,22 @@ func Test_buildResourceIndexFilter(t *testing.T) { }, want: `labels like '%service.name%Application\\\\"%'`, }, + { + name: "test exists", + args: args{ + key: "service.name", + op: v3.FilterOperatorExists, + }, + want: `lower(labels) like '%service.name%'`, + }, + { + name: "test not exists", + args: args{ + key: "service.name", + op: v3.FilterOperatorNotExists, + }, + want: `lower(labels) not like '%service.name%'`, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {