From 13a5c19e67f291209d4864d7aca4379db5171492 Mon Sep 17 00:00:00 2001 From: cncal Date: Tue, 7 Jul 2020 17:10:18 +0800 Subject: [PATCH] enhance #74: builder relax the syntax restriction of orderby operator (#87) * enhance #74: builder relax the syntax restriction of orderby operator * enhance #74:add errOrderByValueType * enhance #85:builder supports uppercase operators (#86) * enhance #74: builder relax the syntax restriction of orderby operator * enhance #74:add errOrderByValueType --- builder/builder.go | 49 +++++++----------------- builder/builder_test.go | 82 +++++++++++++++++++++-------------------- builder/dao.go | 31 ++-------------- builder/dao_test.go | 23 +----------- 4 files changed, 62 insertions(+), 123 deletions(-) diff --git a/builder/builder.go b/builder/builder.go index 1a2a59e..980bd8a 100644 --- a/builder/builder.go +++ b/builder/builder.go @@ -10,10 +10,10 @@ import ( var ( errSplitEmptyKey = errors.New("[builder] couldn't split a empty string") - errSplitOrderBy = errors.New(`[builder] the value of _orderby should be "fieldName direction [,fieldName direction]"`) // ErrUnsupportedOperator reports there's unsupported operators in where-condition ErrUnsupportedOperator = errors.New("[builder] unsupported operator") errOrValueType = errors.New(`[builder] the value of "_or" must be of slice of map[string]interface{} type`) + errOrderByValueType = errors.New(`[builder] the value of "_orderby" must be of string type`) errGroupByValueType = errors.New(`[builder] the value of "_groupby" must be of string type`) errLimitValueType = errors.New(`[builder] the value of "_limit" must be of []uint type`) errLimitValueLength = errors.New(`[builder] the value of "_limit" must contain one or two uint elements`) @@ -40,10 +40,6 @@ func (w *whereMapSet) add(op, field string, val interface{}) { s[field] = val } -type eleOrderBy struct { - field, order string -} - type eleLimit struct { begin, step uint } @@ -52,23 +48,22 @@ type eleLimit struct { // supported operators including: =,in,>,>=,<,<=,<>,!=. // key without operator will be regarded as =. // special key begin with _: _orderby,_groupby,_limit,_having. -// the value of _orderby must be a string separated by a space(ie:map[string]interface{}{"_orderby": "fieldName desc"}). // the value of _limit must be a slice whose type should be []uint and must contain two uints(ie: []uint{0, 100}). // the value of _having must be a map just like where but only support =,in,>,>=,<,<=,<>,!= // for more examples,see README.md or open a issue. func BuildSelect(table string, where map[string]interface{}, selectField []string) (cond string, vals []interface{}, err error) { - var orderBy []eleOrderBy + var orderBy string var limit *eleLimit var groupBy string var having map[string]interface{} copiedWhere := copyWhere(where) if val, ok := copiedWhere["_orderby"]; ok { - eleOrderBy, e := splitOrderBy(val.(string)) - if e != nil { - err = e + s, ok := val.(string) + if !ok { + err = errOrderByValueType return } - orderBy = eleOrderBy + orderBy = strings.TrimSpace(s) delete(copiedWhere, "_orderby") } if val, ok := copiedWhere["_groupby"]; ok { @@ -77,12 +72,14 @@ func BuildSelect(table string, where map[string]interface{}, selectField []strin err = errGroupByValueType return } - groupBy = s + groupBy = strings.TrimSpace(s) delete(copiedWhere, "_groupby") - if h, ok := copiedWhere["_having"]; ok { - having, err = resolveHaving(h) - if nil != err { - return + if "" != groupBy { + if h, ok := copiedWhere["_having"]; ok { + having, err = resolveHaving(h) + if nil != err { + return + } } } } @@ -413,26 +410,6 @@ func removeInnerSpace(operator string) string { return operator[:firstSpace] + operator[lastSpace:] } -func splitOrderBy(orderby string) ([]eleOrderBy, error) { - var err error - var eleOrder []eleOrderBy - for _, val := range strings.Split(orderby, ",") { - val = strings.Trim(val, " ") - idx := strings.IndexByte(val, ' ') - if idx == -1 { - err = errSplitOrderBy - return eleOrder, err - } - field := val[:idx] - direction := strings.Trim(val[idx+1:], " ") - eleOrder = append(eleOrder, eleOrderBy{ - field: field, - order: direction, - }) - } - return eleOrder, err -} - const ( paramPlaceHolder = "?" ) diff --git a/builder/builder_test.go b/builder/builder_test.go index a2c9c58..6f0d790 100644 --- a/builder/builder_test.go +++ b/builder/builder_test.go @@ -121,6 +121,25 @@ func TestBuildHaving(t *testing.T) { err: nil, }, }, + { + in: inStruct{ + table: "tb", + where: map[string]interface{}{ + "_groupby": " ", + "_having": map[string]interface{}{ + "total >=": 1000, + "total <": 50000, + }, + "age in": []interface{}{1, 2, 3}, + }, + selectField: []string{"name, age"}, + }, + out: outStruct{ + cond: "SELECT name, age FROM tb WHERE (age IN (?,?,?))", + vals: []interface{}{1, 2, 3}, + err: nil, + }, + }, } ass := assert.New(t) for _, tc := range data { @@ -369,7 +388,7 @@ func Test_BuildSelect(t *testing.T) { }, }, }, - "_orderby": "age desc, score asc", + "_orderby": "age DESC,score ASC", "_groupby": "department", "_limit": []uint{0, 100}, }, @@ -409,6 +428,21 @@ func Test_BuildSelect(t *testing.T) { err: nil, }, }, + { + in: inStruct{ + table: "tb", + where: map[string]interface{}{ + "foo": "bar", + "_orderby": " ", + }, + fields: nil, + }, + out: outStruct{ + cond: "SELECT * FROM tb WHERE (foo=?)", + vals: []interface{}{"bar"}, + err: nil, + }, + }, } ass := assert.New(t) for _, tc := range data { @@ -426,7 +460,7 @@ func BenchmarkBuildSelect_Sequelization(b *testing.B) { "qq": "tt", "age in": []interface{}{1, 3, 5, 7, 9}, "faith <>": "Muslim", - "_orderby": "age desc", + "_orderby": "age DESC", "_groupby": "department", "_limit": []uint{0, 100}, }, []string{"a", "b", "c"}) @@ -445,7 +479,7 @@ func BenchmarkBuildSelect_Parallel(b *testing.B) { "qq": "tt", "age in": []interface{}{1, 3, 5, 7, 9}, "faith <>": "Muslim", - "_orderby": "age desc", + "_orderby": "age DESC", "_groupby": "department", "_limit": []uint{0, 100}, }, nil) @@ -595,7 +629,7 @@ func Test_BuildIN(t *testing.T) { "qq": "tt", "age in": []int{1, 3, 5, 7, 9}, "faith <>": "Muslim", - "_orderby": "age desc", + "_orderby": "age DESC", "_groupby": "department", }, fields: []string{"id", "name", "age"}, @@ -660,37 +694,7 @@ func Test_BuildOrderBy(t *testing.T) { table: "tb", where: map[string]interface{}{ "foo": "bar", - "_orderby": "age desc, id asc", - }, - fields: []string{"id", "name", "age"}, - }, - out: outStruct{ - cond: "SELECT id,name,age FROM tb WHERE (foo=?) ORDER BY age DESC,id ASC", - vals: []interface{}{"bar"}, - err: nil, - }, - }, - { - in: inStruct{ - table: "tb", - where: map[string]interface{}{ - "foo": "bar", - "_orderby": " age desc,id asc ", - }, - fields: []string{"id", "name", "age"}, - }, - out: outStruct{ - cond: "SELECT id,name,age FROM tb WHERE (foo=?) ORDER BY age DESC,id ASC", - vals: []interface{}{"bar"}, - err: nil, - }, - }, - { - in: inStruct{ - table: "tb", - where: map[string]interface{}{ - "foo": "bar", - "_orderby": " age desc, id asc ", + "_orderby": "age DESC,id ASC", }, fields: []string{"id", "name", "age"}, }, @@ -705,12 +709,12 @@ func Test_BuildOrderBy(t *testing.T) { table: "tb", where: map[string]interface{}{ "foo": "bar", - "_orderby": " age desc", + "_orderby": "RAND()", }, fields: []string{"id", "name", "age"}, }, out: outStruct{ - cond: "SELECT id,name,age FROM tb WHERE (foo=?) ORDER BY age DESC", + cond: "SELECT id,name,age FROM tb WHERE (foo=?) ORDER BY RAND()", vals: []interface{}{"bar"}, err: nil, }, @@ -870,7 +874,7 @@ func Test_NotIn(t *testing.T) { "address": IsNotNull, " hobbies not in ": []string{"baseball", "swim", "running"}, "_groupby": "department", - "_orderby": "bonus desc", + "_orderby": "bonus DESC", }, { "city IN": []string{"beijing", "shanghai"}, @@ -878,7 +882,7 @@ func Test_NotIn(t *testing.T) { "address": IsNotNull, " hobbies NOT IN ": []string{"baseball", "swim", "running"}, "_groupby": "department", - "_orderby": "bonus desc", + "_orderby": "bonus DESC", }, } diff --git a/builder/dao.go b/builder/dao.go index c174fc1..f010d7c 100644 --- a/builder/dao.go +++ b/builder/dao.go @@ -315,23 +315,6 @@ func assembleExpression(field, op string) string { return quoteField(field) + op + "?" } -// caller ensure that orderMap is not empty -func orderBy(orderMap []eleOrderBy) (string, error) { - var str strings.Builder - for _, orderInfo := range orderMap { - realOrder := strings.ToUpper(orderInfo.order) - if realOrder != "ASC" && realOrder != "DESC" { - return "", errOrderByParam - } - str.WriteString(orderInfo.field) - str.WriteByte(' ') - str.WriteString(realOrder) - str.WriteByte(',') - } - finalSQL := str.String() - return finalSQL[:len(finalSQL)-1], nil -} - func resolveKV(m map[string]interface{}) (keys []string, vals []interface{}) { for k := range m { keys = append(keys, k) @@ -452,7 +435,7 @@ func splitCondition(conditions []Comparable) ([]Comparable, []Comparable) { return conditions, nil } -func buildSelect(table string, ufields []string, groupBy string, uOrderBy []eleOrderBy, limit *eleLimit, conditions ...Comparable) (string, []interface{}, error) { +func buildSelect(table string, ufields []string, groupBy, orderBy string, limit *eleLimit, conditions ...Comparable) (string, []interface{}, error) { fields := "*" if len(ufields) > 0 { for i := range ufields { @@ -481,15 +464,9 @@ func buildSelect(table string, ufields []string, groupBy string, uOrderBy []eleO bd.WriteString(havingString) vals = append(vals, havingVals...) } - if len(uOrderBy) != 0 { - str, err := orderBy(uOrderBy) - if nil != err { - return "", nil, err - } - if str != "" { - bd.WriteString(" ORDER BY ") - bd.WriteString(str) - } + if "" != orderBy { + bd.WriteString(" ORDER BY ") + bd.WriteString(orderBy) } if nil != limit { bd.WriteString(" LIMIT ?,?") diff --git a/builder/dao_test.go b/builder/dao_test.go index 0d66748..e71800d 100644 --- a/builder/dao_test.go +++ b/builder/dao_test.go @@ -111,25 +111,6 @@ func TestAssembleExpression(t *testing.T) { } } -func TestOrderBy(t *testing.T) { - var data = []struct { - inOrderBy []eleOrderBy - outStr string - outErr error - }{ - {[]eleOrderBy{eleOrderBy{"age", "desc"}}, "age DESC", nil}, - {[]eleOrderBy{eleOrderBy{"name", "Asc"}}, "name ASC", nil}, - {[]eleOrderBy{eleOrderBy{"tt", "DesC"}}, "tt DESC", nil}, - {[]eleOrderBy{eleOrderBy{"qq", "DESCC"}}, "", errOrderByParam}, - } - ass := assert.New(t) - for _, tc := range data { - actual, err := orderBy(tc.inOrderBy) - ass.Equal(tc.outErr, err) - ass.Equal(tc.outStr, actual) - } -} - func TestResolveKV(t *testing.T) { var data = []struct { in map[string]interface{} @@ -348,7 +329,7 @@ func TestBuildSelect(t *testing.T) { fields []string conditions []Comparable groupBy string - orderBy []eleOrderBy + orderBy string limit *eleLimit outStr string outVals []interface{} @@ -385,7 +366,7 @@ func TestBuildSelect(t *testing.T) { }), }, groupBy: "", - orderBy: []eleOrderBy{eleOrderBy{field: "foo", order: "desc"}, {"baz", "aSc"}}, + orderBy: "foo DESC,baz ASC", limit: &eleLimit{ begin: 10, step: 20,