From 63718cccb7df591934bff6181312c5718a86889b Mon Sep 17 00:00:00 2001 From: Grzegorz Piwowarek Date: Fri, 17 May 2024 16:32:12 +0200 Subject: [PATCH] Enhance 'sort' field parser to handle shorthand maps (#139) Parse shorthand forms of `sort` fields like: ``` "sort": { "@timestamp": "desc", "_doc": "desc" }, ``` --- quesma/queryparser/query_parser.go | 96 ++++++++++++++++--------- quesma/queryparser/query_parser_test.go | 40 +++++++++-- 2 files changed, 96 insertions(+), 40 deletions(-) diff --git a/quesma/queryparser/query_parser.go b/quesma/queryparser/query_parser.go index 696e14ef8..e9379df6d 100644 --- a/quesma/queryparser/query_parser.go +++ b/quesma/queryparser/query_parser.go @@ -82,11 +82,7 @@ func (cw *ClickhouseQueryTranslator) ParseQuery(queryAsJson string) (SimpleQuery } if sortPart, ok := queryAsMap["sort"]; ok { - if sortAsArray, ok := sortPart.([]any); ok { - parsedQuery.SortFields = cw.parseSortFields(sortAsArray) - } else { - logger.WarnWithCtx(cw.Ctx).Msgf("unknown sort format, sort value: %v type: %T", sortPart, sortPart) - } + parsedQuery.SortFields = cw.parseSortFields(sortPart) } const defaultSize = 0 @@ -177,11 +173,7 @@ func (cw *ClickhouseQueryTranslator) ParseQueryAsyncSearch(queryAsJson string) ( } if sort, ok := queryAsMap["sort"]; ok { - if sortAsArray, ok := sort.([]any); ok { - parsedQuery.SortFields = cw.parseSortFields(sortAsArray) - } else { - logger.WarnWithCtx(cw.Ctx).Msgf("unknown sort format, sort value: %v type: %T", sort, sort) - } + parsedQuery.SortFields = cw.parseSortFields(sort) } queryInfo := cw.tryProcessSearchMetadata(queryAsMap) @@ -1146,38 +1138,74 @@ func (cw *ClickhouseQueryTranslator) extractInterval(queryMap QueryMap) string { // parseSortFields parses sort fields from the query // We're skipping ELK internal fields, like "_doc", "_id", etc. (we only accept field starting with "_" if it exists in our table) -func (cw *ClickhouseQueryTranslator) parseSortFields(sortMaps []any) []string { - sortFields := make([]string, 0) - for _, sortMapAsAny := range sortMaps { - sortMap, ok := sortMapAsAny.(QueryMap) - if !ok { - logger.WarnWithCtx(cw.Ctx).Msgf("parseSortFields: unexpected type of value: %T, value: %v", sortMapAsAny, sortMapAsAny) - continue - } - - // sortMap has only 1 key, so we can just iterate over it - for k, v := range sortMap { - if strings.HasPrefix(k, "_") && cw.Table.GetFieldInfo(cw.Ctx, k) == clickhouse.NotExists { - // we're skipping ELK internal fields, like "_doc", "_id", etc. +func (cw *ClickhouseQueryTranslator) parseSortFields(sortMaps any) []string { + switch sortMaps := sortMaps.(type) { + case []any: + sortFields := make([]string, 0) + for _, sortMapAsAny := range sortMaps { + sortMap, ok := sortMapAsAny.(QueryMap) + if !ok { + logger.WarnWithCtx(cw.Ctx).Msgf("parseSortFields: unexpected type of value: %T, value: %v", sortMapAsAny, sortMapAsAny) continue } - fieldName := cw.Table.ResolveField(cw.Ctx, k) - if vAsMap, ok := v.(QueryMap); ok { - if order, ok := vAsMap["order"]; ok { - if orderAsString, ok := order.(string); ok { - sortFields = append(sortFields, strconv.Quote(fieldName)+" "+orderAsString) + + // sortMap has only 1 key, so we can just iterate over it + for k, v := range sortMap { + if strings.HasPrefix(k, "_") && cw.Table.GetFieldInfo(cw.Ctx, k) == clickhouse.NotExists { + // we're skipping ELK internal fields, like "_doc", "_id", etc. + continue + } + fieldName := cw.Table.ResolveField(cw.Ctx, k) + switch v := v.(type) { + case QueryMap: + if order, ok := v["order"]; ok { + if orderAsString, ok := order.(string); ok { + sortFields = append(sortFields, strconv.Quote(fieldName)+" "+orderAsString) + } else { + logger.WarnWithCtx(cw.Ctx).Msgf("unexpected order type: %T, value: %v. Skipping", order, order) + } } else { - logger.WarnWithCtx(cw.Ctx).Msgf("unexpected order type: %T, value: %v. Skipping", order, order) + sortFields = append(sortFields, strconv.Quote(fieldName)) } - } else { - sortFields = append(sortFields, strconv.Quote(fieldName)) + case string: + sortFields = append(sortFields, strconv.Quote(fieldName)+" "+v) + default: + logger.WarnWithCtx(cw.Ctx).Msgf("unexpected 'sort' value's type: %T (key, value): (%s, %v). Skipping", v, k, v) } - } else { - logger.WarnWithCtx(cw.Ctx).Msgf("unexpected value's type: %T (key, value): (%s, %v). Skipping", v, k, v) } } + return sortFields + case map[string]interface{}: + sortFields := make([]string, 0) + + for fieldName, fieldValue := range sortMaps { + if strings.HasPrefix(fieldName, "_") && cw.Table.GetFieldInfo(cw.Ctx, fieldName) == clickhouse.NotExists { + // TODO Elastic internal fields will need to be supported in the future + continue + } + if fieldValue, ok := fieldValue.(string); ok { + sortFields = append(sortFields, fmt.Sprintf("%s %s", strconv.Quote(fieldName), fieldValue)) + } + } + + return sortFields + + case map[string]string: + sortFields := make([]string, 0) + + for fieldName, fieldValue := range sortMaps { + if strings.HasPrefix(fieldName, "_") && cw.Table.GetFieldInfo(cw.Ctx, fieldName) == clickhouse.NotExists { + // TODO Elastic internal fields will need to be supported in the future + continue + } + sortFields = append(sortFields, fmt.Sprintf("%s %s", strconv.Quote(fieldName), fieldValue)) + } + + return sortFields + default: + logger.ErrorWithCtx(cw.Ctx).Msgf("unexpected type of sortMaps: %T, value: %v", sortMaps, sortMaps) + return []string{} } - return sortFields } func (cw *ClickhouseQueryTranslator) parseSize(queryMap QueryMap) (size int, ok bool) { diff --git a/quesma/queryparser/query_parser_test.go b/quesma/queryparser/query_parser_test.go index e4c598c12..12d63c4d7 100644 --- a/quesma/queryparser/query_parser_test.go +++ b/quesma/queryparser/query_parser_test.go @@ -453,22 +453,48 @@ func TestQueryParseDateMathExpression(t *testing.T) { func Test_parseSortFields(t *testing.T) { tests := []struct { - sortMap []any + name string + sortMap any sortFields []string }{ { - []any{ + name: "compound", + sortMap: []any{ QueryMap{"@timestamp": QueryMap{"format": "strict_date_optional_time", "order": "desc", "unmapped_type": "boolean"}}, QueryMap{"service.name": QueryMap{"order": "asc", "unmapped_type": "boolean"}}, QueryMap{"no_order_field": QueryMap{"unmapped_type": "boolean"}}, QueryMap{"_table_field_with_underscore": QueryMap{"order": "asc", "unmapped_type": "boolean"}}, // this should be accepted, as it exists in the table QueryMap{"_doc": QueryMap{"order": "desc", "unmapped_type": "boolean"}}, // this should be discarded, as it doesn't exist in the table }, - []string{`"@timestamp" desc`, `"service.name" asc`, `"no_order_field"`, `"_table_field_with_underscore" asc`}, + sortFields: []string{`"@timestamp" desc`, `"service.name" asc`, `"no_order_field"`, `"_table_field_with_underscore" asc`}, }, { - []any{}, - []string{}, + name: "empty", + sortMap: []any{}, + sortFields: []string{}, + }, + { + name: "map[string]string", + sortMap: map[string]string{ + "timestamp": "desc", + "_doc": "desc", + }, + sortFields: []string{`"timestamp" desc`}, + }, + { + name: "map[string]interface{}", + sortMap: map[string]interface{}{ + "timestamp": "desc", + "_doc": "desc", + }, + sortFields: []string{`"timestamp" desc`}, + }, { + name: "[]map[string]string", + sortMap: []any{ + QueryMap{"@timestamp": "asc"}, + QueryMap{"_doc": "asc"}, + }, + sortFields: []string{`"@timestamp" asc`}, }, } table, _ := clickhouse.NewTable(`CREATE TABLE `+tableName+` @@ -479,6 +505,8 @@ func Test_parseSortFields(t *testing.T) { lm := clickhouse.NewLogManager(concurrent.NewMapWith(tableName, table), config.QuesmaConfiguration{}) cw := ClickhouseQueryTranslator{ClickhouseLM: lm, Table: table, Ctx: context.Background()} for _, tt := range tests { - assert.Equal(t, tt.sortFields, cw.parseSortFields(tt.sortMap)) + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.sortFields, cw.parseSortFields(tt.sortMap)) + }) } }