Skip to content

Commit

Permalink
Support Order By in list API (cadence-workflow#1946)
Browse files Browse the repository at this point in the history 
* Support Order By in list API

* remove manual tests
  • Loading branch information
@vancexu
vancexu authored Jun 4, 2019
1 parent 06cafeb commit c1b5639
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 21 deletions.
19 changes: 9 additions & 10 deletions common/persistence/elasticsearch/esVisibilityStore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -477,11 +477,7 @@ func getESQueryDSLHelper(request *p.ListWorkflowExecutionsRequestV2, token *esVi

isSorted := dsl.Exists(dslFieldSort)
if !isSorted { // set default sorting
if isOpen {
dsl.Set(dslFieldSort, fastjson.MustParse(jsonSortForOpen))
} else {
dsl.Set(dslFieldSort, fastjson.MustParse(jsonSortForClose))
}
dsl.Set(dslFieldSort, fastjson.MustParse(jsonSortForOpen))
} else {
// add WorkflowID as tie-breaker
dsl.Get(dslFieldSort).Set("1", fastjson.MustParse(jsonSortWithTieBreaker))
Expand All @@ -497,20 +493,23 @@ func getESQueryDSLHelper(request *p.ListWorkflowExecutionsRequestV2, token *esVi

func getSQLFromListRequest(request *p.ListWorkflowExecutionsRequestV2) string {
var sql string
if strings.TrimSpace(request.Query) == "" {
sql = fmt.Sprintf("select * from dumy limit %d", request.PageSize)
query := strings.TrimSpace(request.Query)
if query == "" {
sql = fmt.Sprintf("select * from dummy limit %d", request.PageSize)
} else if common.IsJustOrderByClause(query) {
sql = fmt.Sprintf("select * from dummy %s limit %d", request.Query, request.PageSize)
} else {
sql = fmt.Sprintf("select * from dumy where %s limit %d", request.Query, request.PageSize)
sql = fmt.Sprintf("select * from dummy where %s limit %d", request.Query, request.PageSize)
}
return sql
}

func getSQLFromCountRequest(request *p.CountWorkflowExecutionsRequest) string {
var sql string
if strings.TrimSpace(request.Query) == "" {
sql = "select * from dumy"
sql = "select * from dummy"
} else {
sql = fmt.Sprintf("select * from dumy where %s", request.Query)
sql = fmt.Sprintf("select * from dummy where %s", request.Query)
}
return sql
}
Expand Down
20 changes: 13 additions & 7 deletions common/persistence/elasticsearch/esVisibilityStore_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ func (s *ESVisibilitySuite) TestGetESQueryDSL() {
dsl, isOpen, err := getESQueryDSL(request, token)
s.Nil(err)
s.False(isOpen)
s.Equal(`{"query":{"bool":{"must":[{"match_phrase":{"DomainID":{"query":"bfd5c907-f899-4baf-a7b2-2ab85e623ebd"}}},{"bool":{"must":[{"match_all":{}}]}}]}},"from":0,"size":10,"sort":[{"CloseTime":"desc"},{"WorkflowID":"desc"}]}`, dsl)
s.Equal(`{"query":{"bool":{"must":[{"match_phrase":{"DomainID":{"query":"bfd5c907-f899-4baf-a7b2-2ab85e623ebd"}}},{"bool":{"must":[{"match_all":{}}]}}]}},"from":0,"size":10,"sort":[{"StartTime":"desc"},{"WorkflowID":"desc"}]}`, dsl)

request.Query = "invaild query"
dsl, isOpen, err = getESQueryDSL(request, token)
Expand All @@ -674,13 +674,13 @@ func (s *ESVisibilitySuite) TestGetESQueryDSL() {
dsl, isOpen, err = getESQueryDSL(request, token)
s.Nil(err)
s.False(isOpen)
s.Equal(`{"query":{"bool":{"must":[{"match_phrase":{"DomainID":{"query":"bfd5c907-f899-4baf-a7b2-2ab85e623ebd"}}},{"bool":{"must":[{"match_phrase":{"WorkflowID":{"query":"wid"}}}]}}]}},"from":0,"size":10,"sort":[{"CloseTime":"desc"},{"WorkflowID":"desc"}]}`, dsl)
s.Equal(`{"query":{"bool":{"must":[{"match_phrase":{"DomainID":{"query":"bfd5c907-f899-4baf-a7b2-2ab85e623ebd"}}},{"bool":{"must":[{"match_phrase":{"WorkflowID":{"query":"wid"}}}]}}]}},"from":0,"size":10,"sort":[{"StartTime":"desc"},{"WorkflowID":"desc"}]}`, dsl)

request.Query = `WorkflowID = 'wid' or WorkflowID = 'another-wid'`
dsl, isOpen, err = getESQueryDSL(request, token)
s.Nil(err)
s.False(isOpen)
s.Equal(`{"query":{"bool":{"must":[{"match_phrase":{"DomainID":{"query":"bfd5c907-f899-4baf-a7b2-2ab85e623ebd"}}},{"bool":{"should":[{"match_phrase":{"WorkflowID":{"query":"wid"}}},{"match_phrase":{"WorkflowID":{"query":"another-wid"}}}]}}]}},"from":0,"size":10,"sort":[{"CloseTime":"desc"},{"WorkflowID":"desc"}]}`, dsl)
s.Equal(`{"query":{"bool":{"must":[{"match_phrase":{"DomainID":{"query":"bfd5c907-f899-4baf-a7b2-2ab85e623ebd"}}},{"bool":{"should":[{"match_phrase":{"WorkflowID":{"query":"wid"}}},{"match_phrase":{"WorkflowID":{"query":"another-wid"}}}]}}]}},"from":0,"size":10,"sort":[{"StartTime":"desc"},{"WorkflowID":"desc"}]}`, dsl)

request.Query = `WorkflowID = 'wid' order by StartTime desc`
dsl, isOpen, err = getESQueryDSL(request, token)
Expand All @@ -704,19 +704,25 @@ func (s *ESVisibilitySuite) TestGetESQueryDSL() {
dsl, isOpen, err = getESQueryDSL(request, token)
s.Nil(err)
s.False(isOpen)
s.Equal(`{"query":{"bool":{"must":[{"match_phrase":{"DomainID":{"query":"bfd5c907-f899-4baf-a7b2-2ab85e623ebd"}}},{"bool":{"must":[{"match_phrase":{"WorkflowID":{"query":"wid"}}},{"range":{"StartTime":{"gt":"1528383845000000000"}}}]}}]}},"from":0,"size":10,"sort":[{"CloseTime":"desc"},{"WorkflowID":"desc"}]}`, dsl)
s.Equal(`{"query":{"bool":{"must":[{"match_phrase":{"DomainID":{"query":"bfd5c907-f899-4baf-a7b2-2ab85e623ebd"}}},{"bool":{"must":[{"match_phrase":{"WorkflowID":{"query":"wid"}}},{"range":{"StartTime":{"gt":"1528383845000000000"}}}]}}]}},"from":0,"size":10,"sort":[{"StartTime":"desc"},{"WorkflowID":"desc"}]}`, dsl)

request.Query = `ExecutionTime < 1000`
dsl, isOpen, err = getESQueryDSL(request, token)
s.Nil(err)
s.False(isOpen)
s.Equal(`{"query":{"bool":{"must":[{"match_phrase":{"DomainID":{"query":"bfd5c907-f899-4baf-a7b2-2ab85e623ebd"}}},{"bool":{"must":[{"range":{"ExecutionTime":{"gt":"0"}}},{"bool":{"must":[{"range":{"ExecutionTime":{"lt":"1000"}}}]}}]}}]}},"from":0,"size":10,"sort":[{"CloseTime":"desc"},{"WorkflowID":"desc"}]}`, dsl)
s.Equal(`{"query":{"bool":{"must":[{"match_phrase":{"DomainID":{"query":"bfd5c907-f899-4baf-a7b2-2ab85e623ebd"}}},{"bool":{"must":[{"range":{"ExecutionTime":{"gt":"0"}}},{"bool":{"must":[{"range":{"ExecutionTime":{"lt":"1000"}}}]}}]}}]}},"from":0,"size":10,"sort":[{"StartTime":"desc"},{"WorkflowID":"desc"}]}`, dsl)

request.Query = `ExecutionTime < 1000 or ExecutionTime > 2000`
dsl, isOpen, err = getESQueryDSL(request, token)
s.Nil(err)
s.False(isOpen)
s.Equal(`{"query":{"bool":{"must":[{"match_phrase":{"DomainID":{"query":"bfd5c907-f899-4baf-a7b2-2ab85e623ebd"}}},{"bool":{"must":[{"range":{"ExecutionTime":{"gt":"0"}}},{"bool":{"should":[{"range":{"ExecutionTime":{"lt":"1000"}}},{"range":{"ExecutionTime":{"gt":"2000"}}}]}}]}}]}},"from":0,"size":10,"sort":[{"CloseTime":"desc"},{"WorkflowID":"desc"}]}`, dsl)
s.Equal(`{"query":{"bool":{"must":[{"match_phrase":{"DomainID":{"query":"bfd5c907-f899-4baf-a7b2-2ab85e623ebd"}}},{"bool":{"must":[{"range":{"ExecutionTime":{"gt":"0"}}},{"bool":{"should":[{"range":{"ExecutionTime":{"lt":"1000"}}},{"range":{"ExecutionTime":{"gt":"2000"}}}]}}]}}]}},"from":0,"size":10,"sort":[{"StartTime":"desc"},{"WorkflowID":"desc"}]}`, dsl)

request.Query = `order by ExecutionTime desc`
dsl, isOpen, err = getESQueryDSL(request, token)
s.Nil(err)
s.False(isOpen)
s.Equal(`{"query":{"bool":{"must":[{"match_phrase":{"DomainID":{"query":"bfd5c907-f899-4baf-a7b2-2ab85e623ebd"}}},{"bool":{"must":[{"match_all":{}}]}}]}},"from":0,"size":10,"sort":[{"ExecutionTime":"desc"},{"WorkflowID":"desc"}]}`, dsl)

request.Query = `ExecutionTime < "unable to parse"`
_, _, err = getESQueryDSL(request, token)
Expand All @@ -730,7 +736,7 @@ func (s *ESVisibilitySuite) TestGetESQueryDSL() {
dsl, isOpen, err = getESQueryDSL(request, token)
s.Nil(err)
s.False(isOpen)
s.Equal(`{"query":{"bool":{"must":[{"match_phrase":{"DomainID":{"query":"bfd5c907-f899-4baf-a7b2-2ab85e623ebd"}}},{"bool":{"must":[{"match_phrase":{"WorkflowID":{"query":"wid"}}}]}}]}},"from":0,"size":10,"sort":[{"CloseTime":"desc"},{"WorkflowID":"desc"}],"search_after":[1,"a"]}`, dsl)
s.Equal(`{"query":{"bool":{"must":[{"match_phrase":{"DomainID":{"query":"bfd5c907-f899-4baf-a7b2-2ab85e623ebd"}}},{"bool":{"must":[{"match_phrase":{"WorkflowID":{"query":"wid"}}}]}}]}},"from":0,"size":10,"sort":[{"StartTime":"desc"},{"WorkflowID":"desc"}],"search_after":[1,"a"]}`, dsl)
}

func (s *ESVisibilitySuite) TestGetESQueryDSLForScan() {
Expand Down
48 changes: 44 additions & 4 deletions common/queryValidator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ package common

import (
"errors"
"strings"

workflow "github.com/uber/cadence/.gen/go/shared"
"github.com/uber/cadence/common/definition"
Expand Down Expand Up @@ -66,22 +67,39 @@ func (qv *VisibilityQueryValidator) ValidateCountRequestForQuery(countRequest *w
}

// validateListOrCountRequestForQuery valid sql for visibility API
// it also adds attr prefix for customized fields
func (qv *VisibilityQueryValidator) validateListOrCountRequestForQuery(whereClause string) (string, error) {
if len(whereClause) != 0 {
sqlQuery := "SELECT * FROM dummy WHERE " + whereClause
var sqlQuery string
whereClause := strings.TrimSpace(whereClause)
if IsJustOrderByClause(whereClause) { // just order by
sqlQuery = "SELECT * FROM dummy " + whereClause
} else {
sqlQuery = "SELECT * FROM dummy WHERE " + whereClause
}

stmt, err := sqlparser.Parse(sqlQuery)
if err != nil {
return "", &workflow.BadRequestError{Message: "Invalid query."}
}

sel := stmt.(*sqlparser.Select)
err = qv.validateWhereExpr(sel.Where.Expr)
buf := sqlparser.NewTrackedBuffer(nil)
// validate where expr
if sel.Where != nil {
err = qv.validateWhereExpr(sel.Where.Expr)
if err != nil {
return "", &workflow.BadRequestError{Message: err.Error()}
}
sel.Where.Expr.Format(buf)
}
// validate order by
err = qv.validateOrderByExpr(sel.OrderBy)
if err != nil {
return "", &workflow.BadRequestError{Message: err.Error()}
}
sel.OrderBy.Format(buf)

buf := sqlparser.NewTrackedBuffer(nil)
sel.Where.Expr.Format(buf)
return buf.String(), nil
}
return whereClause, nil
Expand Down Expand Up @@ -170,6 +188,28 @@ func (qv *VisibilityQueryValidator) validateRangeExpr(expr sqlparser.Expr) error
return errors.New("invalid search attribute")
}

func (qv *VisibilityQueryValidator) validateOrderByExpr(orderBy sqlparser.OrderBy) error {
for _, orderByExpr := range orderBy {
colName, ok := orderByExpr.Expr.(*sqlparser.ColName)
if !ok {
return errors.New("invalid order by expression")
}
colNameStr := colName.Name.String()
if qv.IsValidSearchAttributes(colNameStr) {
if !definition.IsSystemIndexedKey(colNameStr) { // add search attribute prefix
orderByExpr.Expr = &sqlparser.ColName{
Metadata: colName.Metadata,
Name: sqlparser.NewColIdent(definition.Attr + "." + colNameStr),
Qualifier: colName.Qualifier,
}
}
} else {
return errors.New("invalid order by attribute")
}
}
return nil
}

// IsValidSearchAttributes return true if key is registered
func (qv *VisibilityQueryValidator) IsValidSearchAttributes(key string) bool {
validAttr := qv.validSearchAttributes()
Expand Down
28 changes: 28 additions & 0 deletions common/queryValidator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,32 @@ func (s *queryValidatorSuite) TestValidateListRequestForQuery() {
query = "Invalid between 1 and 2 or WorkflowID = 'wid'"
listRequest.Query = StringPtr(query)
s.Equal("BadRequestError{Message: invalid search attribute}", qv.ValidateListRequestForQuery(listRequest).Error())

// only order by
query = "order by CloseTime desc"
listRequest.Query = StringPtr(query)
s.Nil(qv.ValidateListRequestForQuery(listRequest))
s.Equal(" "+query, listRequest.GetQuery())

// only order by search attribute
query = "order by CustomIntField desc"
listRequest.Query = StringPtr(query)
s.Nil(qv.ValidateListRequestForQuery(listRequest))
s.Equal(" order by `Attr.CustomIntField` desc", listRequest.GetQuery())

// condition + order by
query = "WorkflowID = 'wid' order by CloseTime desc"
listRequest.Query = StringPtr(query)
s.Nil(qv.ValidateListRequestForQuery(listRequest))
s.Equal(query, listRequest.GetQuery())

// invalid order by attribute
query = "order by InvalidField desc"
listRequest.Query = StringPtr(query)
s.Equal("BadRequestError{Message: invalid order by attribute}", qv.ValidateListRequestForQuery(listRequest).Error())

// invalid order by attribute expr
query = "order by 123"
listRequest.Query = StringPtr(query)
s.Equal("BadRequestError{Message: invalid order by expression}", qv.ValidateListRequestForQuery(listRequest).Error())
}
8 changes: 8 additions & 0 deletions common/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"encoding/json"
"fmt"
"math/rand"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -439,3 +440,10 @@ func GetSizeOfMapStringToByteArray(input map[string][]byte) int {
}
return res + golandMapReserverNumberOfBytes
}

// IsJustOrderByClause return true is query start with order by
func IsJustOrderByClause(clause string) bool {
whereClause := strings.TrimSpace(clause)
whereClause = strings.ToLower(whereClause)
return strings.HasPrefix(whereClause, "order by")
}

0 comments on commit c1b5639

Please sign in to comment.