From 2722538e82d683bc8e2b9f69b8205d1a93f0e106 Mon Sep 17 00:00:00 2001 From: Vishal Sharma Date: Wed, 5 Jul 2023 11:20:46 +0530 Subject: [PATCH] Fix/handle hypen attributes (#3023) * fix: handle attributes with hypen `-` * test: update tests * fix: only use backticks on columns orderby --- .../app/traces/v3/query_builder.go | 17 +++-- .../app/traces/v3/query_builder_test.go | 64 +++++++++++-------- 2 files changed, 48 insertions(+), 33 deletions(-) diff --git a/pkg/query-service/app/traces/v3/query_builder.go b/pkg/query-service/app/traces/v3/query_builder.go index abf38b6b40..49572241d9 100644 --- a/pkg/query-service/app/traces/v3/query_builder.go +++ b/pkg/query-service/app/traces/v3/query_builder.go @@ -148,7 +148,9 @@ func buildTracesFilterQuery(fs *v3.FilterSet, keys map[string]v3.AttributeKey) ( return "", fmt.Errorf("invalid value for key %s: %v", item.Key.Key, err) } } - fmtVal = utils.ClickHouseFormattedValue(val) + if val != nil { + fmtVal = utils.ClickHouseFormattedValue(val) + } if operator, ok := tracesOperatorMappingV3[item.Operator]; ok { switch item.Operator { case v3.FilterOperatorContains, v3.FilterOperatorNotContains: @@ -356,7 +358,7 @@ func groupBy(tags ...string) string { func groupByAttributeKeyTags(keys map[string]v3.AttributeKey, tags ...v3.AttributeKey) string { groupTags := []string{} for _, tag := range tags { - groupTags = append(groupTags, tag.Key) + groupTags = append(groupTags, fmt.Sprintf("`%s`", tag.Key)) } return groupBy(groupTags...) } @@ -378,10 +380,10 @@ func orderBy(panelType v3.PanelType, items []v3.OrderBy, tags []string, keys map for _, tag := range tags { if item, ok := itemsLookup[tag]; ok { - orderBy = append(orderBy, fmt.Sprintf("%s %s", item.ColumnName, item.Order)) + orderBy = append(orderBy, fmt.Sprintf("`%s` %s", item.ColumnName, item.Order)) addedToOrderBy[item.ColumnName] = true } else { - orderBy = append(orderBy, fmt.Sprintf("%s ASC", tag)) + orderBy = append(orderBy, fmt.Sprintf("`%s` ASC", tag)) } } @@ -401,7 +403,12 @@ func orderBy(panelType v3.PanelType, items []v3.OrderBy, tags []string, keys map if !addedToOrderBy[item.ColumnName] { attr := v3.AttributeKey{Key: item.ColumnName, DataType: item.DataType, Type: item.Type, IsColumn: item.IsColumn} name := getColumnName(attr, keys) - orderBy = append(orderBy, fmt.Sprintf("%s %s", name, item.Order)) + + if item.IsColumn { + orderBy = append(orderBy, fmt.Sprintf("`%s` %s", name, item.Order)) + } else { + orderBy = append(orderBy, fmt.Sprintf("%s %s", name, item.Order)) + } } } } diff --git a/pkg/query-service/app/traces/v3/query_builder_test.go b/pkg/query-service/app/traces/v3/query_builder_test.go index 97c739d8c1..023a636d12 100644 --- a/pkg/query-service/app/traces/v3/query_builder_test.go +++ b/pkg/query-service/app/traces/v3/query_builder_test.go @@ -340,7 +340,7 @@ var testOrderBy = []struct { }, }, Tags: []string{"name"}, - Result: []string{"name asc", "value desc"}, + Result: []string{"`name` asc", "value desc"}, }, { Name: "Test 2", @@ -356,7 +356,7 @@ var testOrderBy = []struct { }, }, Tags: []string{"name", "bytes"}, - Result: []string{"name asc", "bytes asc"}, + Result: []string{"`name` asc", "`bytes` asc"}, }, { Name: "Test 3", @@ -376,7 +376,7 @@ var testOrderBy = []struct { }, }, Tags: []string{"name", "bytes"}, - Result: []string{"name asc", "bytes asc", "value asc"}, + Result: []string{"`name` asc", "`bytes` asc", "value asc"}, }, { Name: "Test 4", @@ -399,19 +399,27 @@ var testOrderBy = []struct { }, }, Tags: []string{"name", "bytes"}, - Result: []string{"name asc", "bytes asc", "stringTagMap['response_time'] desc"}, + Result: []string{"`name` asc", "`bytes` asc", "stringTagMap['response_time'] desc"}, }, { - Name: "Test 4", + Name: "Test 5", PanelType: v3.PanelTypeList, Items: []v3.OrderBy{ { ColumnName: "name", Order: "asc", + Key: "name", + Type: v3.AttributeKeyTypeTag, + DataType: v3.AttributeKeyDataTypeString, + IsColumn: true, }, { ColumnName: "bytes", Order: "asc", + Key: "bytes", + Type: v3.AttributeKeyTypeTag, + DataType: v3.AttributeKeyDataTypeString, + IsColumn: true, }, { ColumnName: "response_time", @@ -419,7 +427,7 @@ var testOrderBy = []struct { }, }, Tags: []string{}, - Result: []string{"name asc", "bytes asc", "stringTagMap['response_time'] desc"}, + Result: []string{"`name` asc", "`bytes` asc", "stringTagMap['response_time'] desc"}, }, } @@ -630,8 +638,8 @@ var testBuildTracesQueryData = []struct { "toFloat64(count(distinct(name))) as value from signoz_traces.distributed_signoz_index_v2 " + "where (timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000') " + "AND stringTagMap['http.method'] = 'GET' AND resourceTagsMap['x'] != 'abc' " + - "AND has(stringTagMap, 'http.method') group by http.method,ts " + - "order by http.method ASC,ts", + "AND has(stringTagMap, 'http.method') group by `http.method`,ts " + + "order by `http.method` ASC,ts", PanelType: v3.PanelTypeGraph, }, { @@ -662,8 +670,8 @@ var testBuildTracesQueryData = []struct { "toFloat64(count(distinct(name))) as value from signoz_traces.distributed_signoz_index_v2 " + "where (timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000') " + "AND stringTagMap['method'] = 'GET' AND resourceTagsMap['x'] != 'abc' " + - "AND has(stringTagMap, 'method') AND has(resourceTagsMap, 'x') group by method,x,ts " + - "order by method ASC,x ASC,ts", + "AND has(stringTagMap, 'method') AND has(resourceTagsMap, 'x') group by `method`,`x`,ts " + + "order by `method` ASC,`x` ASC,ts", PanelType: v3.PanelTypeGraph, }, { @@ -690,8 +698,8 @@ var testBuildTracesQueryData = []struct { "from signoz_traces.distributed_signoz_index_v2 " + "where (timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000') " + "AND stringTagMap['method'] = 'GET' " + - "AND has(stringTagMap, 'method') group by method,ts " + - "order by method ASC,ts", + "AND has(stringTagMap, 'method') group by `method`,ts " + + "order by `method` ASC,ts", PanelType: v3.PanelTypeGraph, }, { @@ -718,8 +726,8 @@ var testBuildTracesQueryData = []struct { "from signoz_traces.distributed_signoz_index_v2 " + "where (timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000') " + "AND stringTagMap['method'] = 'GET' " + - "AND has(stringTagMap, 'method') group by method,ts " + - "order by method ASC,ts", + "AND has(stringTagMap, 'method') group by `method`,ts " + + "order by `method` ASC,ts", PanelType: v3.PanelTypeGraph, }, { @@ -746,8 +754,8 @@ var testBuildTracesQueryData = []struct { "from signoz_traces.distributed_signoz_index_v2 " + "where (timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000') " + "AND stringTagMap['method'] = 'GET' " + - "AND has(stringTagMap, 'method') group by method,ts " + - "order by method ASC,ts", + "AND has(stringTagMap, 'method') group by `method`,ts " + + "order by `method` ASC,ts", PanelType: v3.PanelTypeGraph, }, { @@ -774,8 +782,8 @@ var testBuildTracesQueryData = []struct { "from signoz_traces.distributed_signoz_index_v2 " + "where (timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000') " + "AND stringTagMap['method'] = 'GET' " + - "AND has(stringTagMap, 'method') group by method,ts " + - "order by method ASC,ts", + "AND has(stringTagMap, 'method') group by `method`,ts " + + "order by `method` ASC,ts", PanelType: v3.PanelTypeGraph, }, { @@ -798,8 +806,8 @@ var testBuildTracesQueryData = []struct { "quantile(0.05)(bytes) as value " + "from signoz_traces.distributed_signoz_index_v2 " + "where (timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000') " + - "AND has(stringTagMap, 'method') group by method,ts " + - "order by method ASC,ts", + "AND has(stringTagMap, 'method') group by `method`,ts " + + "order by `method` ASC,ts", PanelType: v3.PanelTypeGraph, }, { @@ -820,7 +828,7 @@ var testBuildTracesQueryData = []struct { ExpectedQuery: "SELECT toStartOfInterval(timestamp, INTERVAL 60 SECOND) AS ts, stringTagMap['method'] as `method`" + ", sum(bytes)/60 as value from signoz_traces.distributed_signoz_index_v2 " + "where (timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000')" + - " AND has(stringTagMap, 'method') group by method,ts order by method ASC,ts", + " AND has(stringTagMap, 'method') group by `method`,ts order by `method` ASC,ts", PanelType: v3.PanelTypeGraph, }, { @@ -841,8 +849,8 @@ var testBuildTracesQueryData = []struct { ExpectedQuery: "SELECT toStartOfInterval(timestamp, INTERVAL 60 SECOND) AS ts, stringTagMap['method'] as `method`" + ", count(numberTagMap['bytes'])/60 as value " + "from signoz_traces.distributed_signoz_index_v2 where (timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000') " + - "AND has(stringTagMap, 'method') group by method,ts " + - "order by method ASC,ts", + "AND has(stringTagMap, 'method') group by `method`,ts " + + "order by `method` ASC,ts", PanelType: v3.PanelTypeGraph, }, { @@ -864,8 +872,8 @@ var testBuildTracesQueryData = []struct { "stringTagMap['method'] as `method`, " + "sum(numberTagMap['bytes'])/60 as value " + "from signoz_traces.distributed_signoz_index_v2 where (timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000') " + - "AND has(stringTagMap, 'method') group by method,ts " + - "order by method ASC,ts", + "AND has(stringTagMap, 'method') group by `method`,ts " + + "order by `method` ASC,ts", PanelType: v3.PanelTypeGraph, }, { @@ -984,7 +992,7 @@ var testBuildTracesQueryData = []struct { }, ExpectedQuery: "SELECT timestamp as timestamp_datetime, spanID, traceID," + " name as `name` from signoz_traces.distributed_signoz_index_v2 where " + - "(timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000') order by name ASC", + "(timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000') order by `name` ASC", PanelType: v3.PanelTypeList, }, { @@ -1006,8 +1014,8 @@ var testBuildTracesQueryData = []struct { }, ExpectedQuery: "SELECT timestamp as timestamp_datetime, spanID, traceID," + " name as `name` from signoz_traces.distributed_signoz_index_v2 where " + - "(timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000')" + - " AND stringTagMap['method'] = 'GET' order by name ASC", + "(timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000')" + + " AND stringTagMap['method'] = 'GET' order by `name` ASC", PanelType: v3.PanelTypeList, }, {