Skip to content

Commit

Permalink
Typed SortFields instead of concatenated string
Browse files Browse the repository at this point in the history
  • Loading branch information
@pivovarit
pivovarit committed May 20, 2024
1 parent b9c7e96 commit b1bba6c
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 33 deletions.
20 changes: 18 additions & 2 deletions quesma/eql/query_translator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (cw *ClickhouseEQLQueryTranslator) applySizeLimit(size int) int {
func (cw *ClickhouseEQLQueryTranslator) BuildNRowsQuery(fieldName string, simpleQuery queryparser.SimpleQuery, limit int) *model.Query {
suffixClauses := make([]string, 0)
if len(simpleQuery.SortFields) > 0 {
suffixClauses = append(suffixClauses, "ORDER BY "+strings.Join(simpleQuery.SortFields, ", "))
suffixClauses = append(suffixClauses, "ORDER BY "+asQueryString(simpleQuery.SortFields))
}
if limit > 0 {
suffixClauses = append(suffixClauses, "LIMIT "+strconv.Itoa(cw.applySizeLimit(limit)))
Expand Down Expand Up @@ -143,7 +143,7 @@ func (cw *ClickhouseEQLQueryTranslator) ParseQuery(queryAsJson string) (query qu

query.Sql.Stmt = where
query.CanParse = true
query.SortFields = []string{"\"@timestamp\""}
query.SortFields = []queryparser.SortField{{Field: "@timestamp"}}

return query, searchQueryInfo, highlighter, nil
}
Expand All @@ -165,3 +165,19 @@ func (cw *ClickhouseEQLQueryTranslator) BuildFacetsQuery(fieldName string, simpl
func (cw *ClickhouseEQLQueryTranslator) ParseAggregationJson(aggregationJson string) ([]model.Query, error) {
panic("EQL does not support aggregations")
}

func asQueryString(sortFields []queryparser.SortField) string {
if len(sortFields) == 0 {
return ""
}
sortStrings := make([]string, 0, len(sortFields))
for _, sortField := range sortFields {
query := strings.Builder{}
query.WriteString(strconv.Quote(sortField.Field))
if sortField.Desc {
query.WriteString(" desc")
}
sortStrings = append(sortStrings, query.String())
}
return strings.Join(sortStrings, ", ")
}
68 changes: 45 additions & 23 deletions quesma/queryparser/query_parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,24 @@ import (

type QueryMap = map[string]interface{}

type SimpleQuery struct {
Sql Statement
CanParse bool
FieldName string
SortFields []string
}
type (
SimpleQuery struct {
Sql Statement
CanParse bool
FieldName string
SortFields []SortField
}
Statement struct {
Stmt string
isCompound bool // "a" -> not compound, "a AND b" -> compound. Used to not make unnecessary brackets (not always, but usually)
FieldName string
}

type Statement struct {
Stmt string
isCompound bool // "a" -> not compound, "a AND b" -> compound. Used to not make unnecessary brackets (not always, but usually)
FieldName string
}
SortField struct {
Field string
Desc bool
}
)

// Added to the generated SQL where the query is fine, but we're sure no rows will match it
var alwaysFalseStatement = NewSimpleStatement("false")
Expand Down Expand Up @@ -1143,10 +1149,10 @@ 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 {
func (cw *ClickhouseQueryTranslator) parseSortFields(sortMaps any) (sortFields []SortField) {
sortFields = make([]SortField, 0)
switch sortMaps := sortMaps.(type) {
case []any:
sortFields := make([]string, 0)
for _, sortMapAsAny := range sortMaps {
sortMap, ok := sortMapAsAny.(QueryMap)
if !ok {
Expand All @@ -1165,51 +1171,67 @@ func (cw *ClickhouseQueryTranslator) parseSortFields(sortMaps any) []string {
case QueryMap:
if order, ok := v["order"]; ok {
if orderAsString, ok := order.(string); ok {
sortFields = append(sortFields, strconv.Quote(fieldName)+" "+orderAsString)
orderAsString = strings.ToLower(orderAsString)
if orderAsString == "asc" || orderAsString == "desc" {
sortFields = append(sortFields, SortField{Field: fieldName, Desc: orderAsString == "desc"})
} else {
logger.WarnWithCtx(cw.Ctx).Msgf("unexpected order value: %s. Skipping", orderAsString)
}
} else {
logger.WarnWithCtx(cw.Ctx).Msgf("unexpected order type: %T, value: %v. Skipping", order, order)
}
} else {
sortFields = append(sortFields, strconv.Quote(fieldName))
sortFields = append(sortFields, SortField{Field: fieldName, Desc: false})
}
case string:
sortFields = append(sortFields, strconv.Quote(fieldName)+" "+v)
v = strings.ToLower(v)
if v == "asc" || v == "desc" {
sortFields = append(sortFields, SortField{Field: fieldName, Desc: v == "desc"})
} else {
logger.WarnWithCtx(cw.Ctx).Msgf("unexpected order value: %s. Skipping", v)
}
default:
logger.WarnWithCtx(cw.Ctx).Msgf("unexpected 'sort' 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))
fieldValue = strings.ToLower(fieldValue)
if fieldValue == "asc" || fieldValue == "desc" {
sortFields = append(sortFields, SortField{Field: fieldName, Desc: fieldValue == "desc"})
} else {
logger.WarnWithCtx(cw.Ctx).Msgf("unexpected order value: %s. Skipping", 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))
fieldValue = strings.ToLower(fieldValue)
if fieldValue == "asc" || fieldValue == "desc" {
sortFields = append(sortFields, SortField{Field: fieldName, Desc: fieldValue == "desc"})
} else {
logger.WarnWithCtx(cw.Ctx).Msgf("unexpected order value: %s. Skipping", fieldValue)
}
}

return sortFields
default:
logger.ErrorWithCtx(cw.Ctx).Msgf("unexpected type of sortMaps: %T, value: %v", sortMaps, sortMaps)
return []string{}
return []SortField{}
}
}

Expand Down
23 changes: 17 additions & 6 deletions quesma/queryparser/query_parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ func Test_parseSortFields(t *testing.T) {
tests := []struct {
name string
sortMap any
sortFields []string
sortFields []SortField
}{
{
name: "compound",
Expand All @@ -454,35 +454,46 @@ func Test_parseSortFields(t *testing.T) {
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
},
sortFields: []string{`"@timestamp" desc`, `"service.name" asc`, `"no_order_field"`, `"_table_field_with_underscore" asc`},
sortFields: []SortField{
{Field: "@timestamp", Desc: true},
{Field: "service.name", Desc: false},
{Field: "no_order_field", Desc: false},
{Field: "_table_field_with_underscore", Desc: false},
},
},
{
name: "empty",
sortMap: []any{},
sortFields: []string{},
sortFields: []SortField{},
},
{
name: "map[string]string",
sortMap: map[string]string{
"timestamp": "desc",
"_doc": "desc",
},
sortFields: []string{`"timestamp" desc`},
sortFields: []SortField{
{Field: "timestamp", Desc: true},
},
},
{
name: "map[string]interface{}",
sortMap: map[string]interface{}{
"timestamp": "desc",
"_doc": "desc",
},
sortFields: []string{`"timestamp" desc`},
sortFields: []SortField{
{Field: "timestamp", Desc: true},
},
}, {
name: "[]map[string]string",
sortMap: []any{
QueryMap{"@timestamp": "asc"},
QueryMap{"_doc": "asc"},
},
sortFields: []string{`"@timestamp" asc`},
sortFields: []SortField{
{Field: "@timestamp", Desc: false},
},
},
}
table, _ := clickhouse.NewTable(`CREATE TABLE `+tableName+`
Expand Down
18 changes: 17 additions & 1 deletion quesma/queryparser/query_translator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ func (cw *ClickhouseQueryTranslator) applySizeLimit(size int) int {
func (cw *ClickhouseQueryTranslator) BuildNRowsQuery(fieldName string, query SimpleQuery, limit int) *model.Query {
suffixClauses := make([]string, 0)
if len(query.SortFields) > 0 {
suffixClauses = append(suffixClauses, "ORDER BY "+strings.Join(query.SortFields, ", "))
suffixClauses = append(suffixClauses, "ORDER BY "+asQueryString(query.SortFields))
}
if limit > 0 {
suffixClauses = append(suffixClauses, "LIMIT "+strconv.Itoa(cw.applySizeLimit(limit)))
Expand Down Expand Up @@ -728,3 +728,19 @@ func (cw *ClickhouseQueryTranslator) sortInTopologicalOrder(queries []model.Quer
}
return indexesSorted
}

func asQueryString(sortFields []SortField) string {
if len(sortFields) == 0 {
return ""
}
sortStrings := make([]string, 0, len(sortFields))
for _, sortField := range sortFields {
query := strings.Builder{}
query.WriteString(strconv.Quote(sortField.Field))
if sortField.Desc {
query.WriteString(" desc")
}
sortStrings = append(sortStrings, query.String())
}
return strings.Join(sortStrings, ", ")
}
2 changes: 1 addition & 1 deletion quesma/quesma/search.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ func (q *QueryRunner) findNonexistingProperties(queryInfo model.SearchQueryInfo,
var allReferencedFields = make([]string, 0)
allReferencedFields = append(allReferencedFields, queryInfo.RequestedFields...)
for _, field := range simpleQuery.SortFields {
allReferencedFields = append(allReferencedFields, strings.ReplaceAll(strings.Fields(field)[0], `"`, ""))
allReferencedFields = append(allReferencedFields, field.Field)
}

for _, property := range allReferencedFields {
Expand Down

0 comments on commit b1bba6c

Please sign in to comment.