Merge branch 'fix-ql-nesting' into develop

pkg/ql/ast_parser.go

@@ -193,12 +193,21 @@ checkToken:
 		list = append(list, String{Value: t.literal})
 		goto next
 	case PARENTHESIS_OPEN:
+		depth := p.level
 		p.level++
 		if sub, err := p.parseExpr(p.nextToken()); err != nil {
 			return nil, err
 		} else {
 			list = append(list, sub)
 		}
+
+		// Allow parent level to continue parsing.
+		// Example: ((A) AND (B))
+		// +1 since PARENTHESIS_CLOSE decrease level
+		if (p.level + 1) != depth {
+			p.nextToken()
+			goto next
+		}
 	default:
 		return nil, fmt.Errorf("unexpected token while parsing expression (%v)", t)
 	}

pkg/ql/ast_parser_test.go

@@ -183,6 +183,39 @@ func TestAstParser_Parser(t *testing.T) {
 				Null{},
 			},
 		},
+		{
+			parser: NewParser().ParseExpression,
+			in:     `((foo1) AND (foo2))`,
+			tree: ASTNodes{
+				ASTNodes{Ident{Value: "foo1"}},
+				Operator{"AND"},
+				ASTNodes{Ident{Value: "foo2"}},
+			},
+		},
+		{
+			parser: NewParser().ParseExpression,
+			in:     `((foo1) AND (foo2) AND foo3)`,
+			tree: ASTNodes{
+				ASTNodes{Ident{Value: "foo1"}},
+				Operator{"AND"},
+				ASTNodes{Ident{Value: "foo2"}},
+				Operator{"AND"},
+				Ident{Value: "foo3"},
+			},
+		},
+		{
+			parser: NewParser().ParseExpression,
+			in:     `((foo1) AND (foo2)) AND foo3`,
+			tree: ASTNodes{
+				ASTNodes{
+					ASTNodes{Ident{Value: "foo1"}},
+					Operator{"AND"},
+					ASTNodes{Ident{Value: "foo2"}},
+				},
+				Operator{"AND"},
+				Ident{Value: "foo3"},
+			},
+		},
 	}
 
 	for i, test := range tests {

pkg/ql/squirrel.go

@@ -18,7 +18,13 @@ func (nn ASTNodes) ToSql() (out string, args []interface{}, err error) {
 		if _out, _args, err = s.ToSql(); err != nil {
 			return
 		} else {
-			out = out + _out
+			if _, ok := s.(ASTNodes); ok {
+				// Nested nodes should be wrapped
+				// Example: ((A) AND (B))
+				out = out + "(" + _out + ")"
+			} else {
+				out = out + _out
+			}
 
 			args = append(args, _args...)
 		}

tests/compose/record_exec_test.go

@@ -57,7 +57,7 @@ func TestRecordExec(t *testing.T) {
 		)
 	}
 
-	assertSort := func(expectedHandles string) {
+	assertSort := func(expectedHandles, expectedCats string) {
 		// Using record service for fetching to avoid value pre-fetching etc..
 		set, _, err := service.DefaultRecord.With(h.secCtx()).Find(types.RecordFilter{
 			ModuleID:    module.ID,
@@ -70,6 +70,8 @@ func TestRecordExec(t *testing.T) {
 		h.a.NotNil(set)
 
 		actualHandles := ""
+		actualCats := ""
+
 		_ = set.Walk(func(r *types.Record) error {
 			v := r.Values.FilterByName("handle")
 
@@ -79,10 +81,13 @@ func TestRecordExec(t *testing.T) {
 				actualHandles += strconv.Itoa(len(v))
 			}
 
+			actualCats += r.Values.FilterByName("category")[0].Value[3:]
+
 			return nil
 		})
 
 		h.a.Equal(expectedHandles, actualHandles)
+		h.a.Equal(expectedCats, actualCats)
 	}
 
 	var (
@@ -110,8 +115,9 @@ func TestRecordExec(t *testing.T) {
 		"i": strconv.FormatUint(iRec.ID, 10),
 	}
 
-	assertSort("abcdefghi")
+	assertSort("abcdefghi", "111222333")
 
+	// Move a to the middle
 	h.apiSendRecordExec(module.NamespaceID, module.ID, "organize", request.ProcedureArgs{
 		{"recordID", rr["a"]},
 		{"positionField", "position"},
@@ -120,8 +126,9 @@ func TestRecordExec(t *testing.T) {
 		Assert(helpers.AssertNoErrors).
 		End()
 
-	assertSort("bcdeafghi")
+	assertSort("bcdeafghi", "112212333")
 
+	// Move i to the beginning
 	h.apiSendRecordExec(module.NamespaceID, module.ID, "organize", request.ProcedureArgs{
 		{"recordID", rr["i"]},
 		{"positionField", "position"},
@@ -130,8 +137,11 @@ func TestRecordExec(t *testing.T) {
 		Assert(helpers.AssertNoErrors).
 		End()
 
-	assertSort("ibcdeafgh")
+	//                            bcdeafghi
+	//                            v<------^
+	assertSort("ibcdeafgh", "311221233")
 
+	// Move b to the 5th place
 	h.apiSendRecordExec(module.NamespaceID, module.ID, "organize", request.ProcedureArgs{
 		{"recordID", rr["b"]},
 		{"filter", "category = 'CAT1'"},
@@ -141,8 +151,11 @@ func TestRecordExec(t *testing.T) {
 		Assert(helpers.AssertNoErrors).
 		End()
 
-	assertSort("idecbfgah")
+	//                            ibcdeafgh
+	//                             ^->v
+	assertSort("icdebfagh", "312212133")
 
+	// This will keep order of letters but move b to category-2
 	h.apiSendRecordExec(module.NamespaceID, module.ID, "organize", request.ProcedureArgs{
 		{"recordID", rr["b"]},
 		{"groupField", "category"},
@@ -152,7 +165,10 @@ func TestRecordExec(t *testing.T) {
 		Assert(helpers.AssertNoErrors).
 		End()
 
-	assertSort("idecbfgah")
+	//                            icdebfagh
+	//                                ^
+	assertSort("icdebfagh", "312222133")
+
 	rsv, err := h.repoRecord().LoadValues([]string{"category"}, []uint64{bRec.ID})
 	h.a.NoError(err)
 	h.a.NotNil(rsv)