Repository: genjidb/genji
Pull request #131 (2020-07-19 13:24:31)

Fix parentheses behaviour
This PR changes how parentheses were parsed by the parser. Previously, they were considered as lists of expressions, which after evaluation turned into an array. This behaviour prevented us from using parentheses as arithmetic sub expressions.

Before:
```sql
genji> select ((10 + 5 + 5) * 4) / 2;
{
  "((10 + 5 + 5) * 4) / 1": null
}
genji> select ((10 + 5 + 5) * 4);
{
  "((10 + 5 + 5) * 4)": [
    null
  ]
}
```

After:
```sql
genji> select ((10 + 5 + 5) * 4) / 2;
{
  "((10 + 5 + 5) * 4) / 1": 40
}
genji> select ((10 + 5 + 5) * 4);
{
  "((10 + 5 + 5) * 4)": 80
}
```


Patch:
diff --git a/docs/themes/docsy b/docs/themes/docsy
index 053672c2..02904c33 160000
--- a/docs/themes/docsy
+++ b/docs/themes/docsy
@@ -1,1 +1,1 @@
-Subproject commit 053672c24cf51417c300f8ea5d32db545340746d
+Subproject commit 02904c33475a4a9e21104bff901f8c6f821fd597
diff --git a/sql/parser/expr.go b/sql/parser/expr.go
index 7845ccb2..04b70854 100644
--- a/sql/parser/expr.go
+++ b/sql/parser/expr.go
@@ -209,8 +209,14 @@ func (p *Parser) parseUnaryExpr() (expr.Expr, error) {
 		p.Unscan()
 		return p.parseExprList(scanner.LSBRACKET, scanner.RSBRACKET)
 	case scanner.LPAREN:
-		p.Unscan()
-		return p.parseExprList(scanner.LPAREN, scanner.RPAREN)
+		e, _, err := p.ParseExpr()
+		if err != nil {
+			return nil, err
+		}
+		if tok, pos, lit := p.ScanIgnoreWhitespace(); tok != scanner.RPAREN {
+			return nil, newParseError(scanner.Tokstr(tok, lit), []string{")"}, pos)
+		}
+		return expr.Parentheses{E: e}, nil
 	default:
 		return nil, newParseError(scanner.Tokstr(tok, lit), []string{"identifier", "string", "number", "bool"}, pos)
 	}
diff --git a/sql/query/expr/expr.go b/sql/query/expr/expr.go
index 3a7d37bf..d22de393 100644
--- a/sql/query/expr/expr.go
+++ b/sql/query/expr/expr.go
@@ -119,3 +119,16 @@ type Operator interface {
 	SetRightHandExpr(Expr)
 	Token() scanner.Token
 }
+
+// Parentheses is a special expression which turns
+// any sub-expression as unary.
+// It hides the underlying operator, if any, from the parser
+// so that it doesn't get reordered by precedence.
+type Parentheses struct {
+	E Expr
+}
+
+// Eval calls the underlying expression Eval method.
+func (p Parentheses) Eval(es EvalStack) (document.Value, error) {
+	return p.E.Eval(es)
+}


Test patch:
diff --git a/sql/parser/expr_test.go b/sql/parser/expr_test.go
index f76cc2fe..7dcbb7c3 100644
--- a/sql/parser/expr_test.go
+++ b/sql/parser/expr_test.go
@@ -87,18 +87,27 @@ func TestParserExpr(t *testing.T) {
 		{"bad document: missing right bracket", `{a: 1`, nil, true},
 		{"bad document: missing colon", `{a: 1, 'b'}`, nil, true},
 
-		// list of expressions
-		{"list with parentheses: empty", "()", expr.LiteralExprList(nil), false},
-		{"list with parentheses: values", `(1, true, {a: 1}, a.b.c, (-1), [-1])`,
-			expr.LiteralExprList{
-				expr.IntegerValue(1),
-				expr.BoolValue(true),
-				expr.KVPairs{expr.KVPair{K: "a", V: expr.IntegerValue(1)}},
-				expr.FieldSelector{"a", "b", "c"},
-				expr.LiteralExprList{expr.IntegerValue(-1)},
-				expr.LiteralExprList{expr.IntegerValue(-1)},
+		// parentheses
+		{"parentheses: empty", "()", nil, true},
+		{"parentheses: values", `(1)`,
+			expr.Parentheses{
+				E: expr.IntegerValue(1),
+			}, false},
+		{"parentheses: expr", `(1 + true * (4 + 3))`,
+			expr.Parentheses{
+				E: expr.Add(
+					expr.IntegerValue(1),
+					expr.Mul(
+						expr.BoolValue(true),
+						expr.Parentheses{
+							E: expr.Add(
+								expr.IntegerValue(4),
+								expr.IntegerValue(3),
+							),
+						},
+					),
+				),
 			}, false},
-		{"list with parentheses: missing parenthese", `(1, true, {a: 1}, a.b.c, (-1)`, nil, true},
 		{"list with brackets: empty", "[]", expr.LiteralExprList(nil), false},
 		{"list with brackets: values", `[1, true, {a: 1}, a.b.c, (-1), [-1]]`,
 			expr.LiteralExprList{
@@ -106,7 +115,7 @@ func TestParserExpr(t *testing.T) {
 				expr.BoolValue(true),
 				expr.KVPairs{expr.KVPair{K: "a", V: expr.IntegerValue(1)}},
 				expr.FieldSelector{"a", "b", "c"},
-				expr.LiteralExprList{expr.IntegerValue(-1)},
+				expr.Parentheses{E: expr.IntegerValue(-1)},
 				expr.LiteralExprList{expr.IntegerValue(-1)},
 			}, false},
 		{"list with brackets: missing bracket", `[1, true, {a: 1}, a.b.c, (-1), [-1]`, nil, true},
diff --git a/sql/query/insert_test.go b/sql/query/insert_test.go
index bc15b772..28efae6f 100644
--- a/sql/query/insert_test.go
+++ b/sql/query/insert_test.go
@@ -28,11 +28,11 @@ func TestInsertStmt(t *testing.T) {
 		{"Values / Positional Params", "INSERT INTO test (a, b, c) VALUES (?, 'e', ?)", false, `{"pk()":1,"a":"d","b":"e","c":"f"}`, []interface{}{"d", "f"}},
 		{"Values / Named Params", "INSERT INTO test (a, b, c) VALUES ($d, 'e', $f)", false, `{"pk()":1,"a":"d","b":"e","c":"f"}`, []interface{}{sql.Named("f", "f"), sql.Named("d", "d")}},
 		{"Values / Invalid params", "INSERT INTO test (a, b, c) VALUES ('d', ?)", true, "", []interface{}{'e'}},
-		{"Values / List", `INSERT INTO test (a, b, c) VALUES ("a", 'b', (1, 2, 3))`, false, `{"pk()":1,"a":"a","b":"b","c":[1,2,3]}`, nil},
+		{"Values / List", `INSERT INTO test (a, b, c) VALUES ("a", 'b', [1, 2, 3])`, false, `{"pk()":1,"a":"a","b":"b","c":[1,2,3]}`, nil},
 		{"Documents", "INSERT INTO test VALUES {a: 'a', b: 2.3, c: 1 = 1}", false, `{"pk()":1,"a":"a","b":2.3,"c":true}`, nil},
 		{"Documents / Positional Params", "INSERT INTO test VALUES {a: ?, b: 2.3, c: ?}", false, `{"pk()":1,"a":"a","b":2.3,"c":true}`, []interface{}{"a", true}},
 		{"Documents / Named Params", "INSERT INTO test VALUES {a: $a, b: 2.3, c: $c}", false, `{"pk()":1,"a":1,"b":2.3,"c":true}`, []interface{}{sql.Named("c", true), sql.Named("a", 1)}},
-		{"Documents / List ", "INSERT INTO test VALUES {a: (1, 2, 3)}", false, `{"pk()":1,"a":[1,2,3]}`, nil},
+		{"Documents / List ", "INSERT INTO test VALUES {a: [1, 2, 3]}", false, `{"pk()":1,"a":[1,2,3]}`, nil},
 		{"Documents / strings", `INSERT INTO test VALUES {'a': 'a', b: 2.3}`, false, `{"pk()":1,"a":"a","b":2.3}`, nil},
 		{"Documents / double quotes", `INSERT INTO test VALUES {"a": "b"}`, false, `{"pk()":1,"a":"b"}`, nil},
 	}