Skip to content

Commit b7ff0e6

Browse files
ostafenostafen
committed
fix(embedded/sql): correctly handle logical operator precedence (NOT, AND, OR)
Signed-off-by: Stefano Scafiti <[email protected]>
1 parent 344275d commit b7ff0e6

File tree

7 files changed

+312
-254
lines changed

7 files changed

+312
-254
lines changed

‎embedded/document/engine.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1175,14 +1175,14 @@ func generateSQLFilteringExpression(expressions []*protomodel.QueryExpression, t
11751175
if i == 0 {
11761176
innerExp = fieldExp
11771177
} else {
1178-
innerExp = sql.NewBinBoolExp(sql.AND, innerExp, fieldExp)
1178+
innerExp = sql.NewBinBoolExp(sql.And, innerExp, fieldExp)
11791179
}
11801180
}
11811181

11821182
if i == 0 {
11831183
outerExp = innerExp
11841184
} else {
1185-
outerExp = sql.NewBinBoolExp(sql.OR, outerExp, innerExp)
1185+
outerExp = sql.NewBinBoolExp(sql.Or, outerExp, innerExp)
11861186
}
11871187
}
11881188

‎embedded/sql/parser.go‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ var reservedWords = map[string]int{
8383
"AS": AS,
8484
"ASC": ASC,
8585
"DESC": DESC,
86+
"AND": AND,
87+
"OR": OR,
8688
"NOT": NOT,
8789
"LIKE": LIKE,
8890
"EXISTS": EXISTS,
@@ -157,11 +159,6 @@ var cmpOps = map[string]CmpOperator{
157159
">=": GE,
158160
}
159161

160-
var logicOps = map[string]LogicOperator{
161-
"AND": AND,
162-
"OR": OR,
163-
}
164-
165162
var ErrEitherNamedOrUnnamedParams = errors.New("either named or unnamed params")
166163
var ErrEitherPosOrNonPosParams = errors.New("either positional or non-positional named params")
167164
var ErrInvalidPositionalParameter = errors.New("invalid positional parameter")
@@ -348,12 +345,6 @@ func (l *lexer) Lex(lval *yySymType) int {
348345
return BOOLEAN
349346
}
350347

351-
lop, ok := logicOps[tid]
352-
if ok {
353-
lval.logicOp = lop
354-
return LOP
355-
}
356-
357348
afn, ok := aggregateFns[tid]
358349
if ok {
359350
lval.aggFn = afn

‎embedded/sql/parser_test.go‎

Lines changed: 60 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ func TestInsertIntoStmt(t *testing.T) {
659659
},
660660
targets: nil,
661661
where: &BinBoolExp{
662-
op: AND,
662+
op: And,
663663
left: &CmpBoolExp{op: GE, left: &ColSelector{col: "balance"}, right: &Integer{val: 0}},
664664
right: &CmpBoolExp{op: EQ, left: &ColSelector{col: "deleted_at"}, right: &NullValue{t: AnyType}},
665665
},
@@ -1009,9 +1009,9 @@ func TestSelectStmt(t *testing.T) {
10091009
},
10101010
ds: &tableRef{table: "table1"},
10111011
where: &BinBoolExp{
1012-
op: AND,
1012+
op: And,
10131013
left: &BinBoolExp{
1014-
op: AND,
1014+
op: And,
10151015
left: &CmpBoolExp{
10161016
op: EQ,
10171017
left: &ColSelector{
@@ -1208,7 +1208,7 @@ func TestSelectStmt(t *testing.T) {
12081208
},
12091209
ds: &tableRef{table: "table1"},
12101210
where: &BinBoolExp{
1211-
op: AND,
1211+
op: And,
12121212
left: &CmpBoolExp{
12131213
op: GE,
12141214
left: &ColSelector{
@@ -1421,7 +1421,7 @@ func TestParseExp(t *testing.T) {
14211421
},
14221422
ds: &tableRef{table: "table1"},
14231423
where: &BinBoolExp{
1424-
op: AND,
1424+
op: And,
14251425
left: &NotBoolExp{
14261426
exp: &CmpBoolExp{
14271427
op: GT,
@@ -1453,7 +1453,7 @@ func TestParseExp(t *testing.T) {
14531453
ds: &tableRef{table: "table1"},
14541454
where: &NotBoolExp{
14551455
exp: &BinBoolExp{
1456-
op: AND,
1456+
op: And,
14571457
left: &CmpBoolExp{
14581458
op: GT,
14591459
left: &ColSelector{
@@ -1483,7 +1483,7 @@ func TestParseExp(t *testing.T) {
14831483
},
14841484
ds: &tableRef{table: "table1"},
14851485
where: &BinBoolExp{
1486-
op: OR,
1486+
op: Or,
14871487
left: &NotBoolExp{
14881488
exp: &ColSelector{col: "active"},
14891489
},
@@ -1502,7 +1502,7 @@ func TestParseExp(t *testing.T) {
15021502
},
15031503
ds: &tableRef{table: "table1"},
15041504
where: &BinBoolExp{
1505-
op: AND,
1505+
op: And,
15061506
left: &CmpBoolExp{
15071507
op: GT,
15081508
left: &ColSelector{
@@ -1572,9 +1572,9 @@ func TestParseExp(t *testing.T) {
15721572
},
15731573
ds: &tableRef{table: "table1"},
15741574
where: &BinBoolExp{
1575-
op: OR,
1575+
op: Or,
15761576
left: &BinBoolExp{
1577-
op: AND,
1577+
op: And,
15781578
left: &CmpBoolExp{
15791579
op: GT,
15801580
left: &ColSelector{
@@ -1711,7 +1711,7 @@ func TestParseExp(t *testing.T) {
17111711
whenThen: []whenThenClause{
17121712
{
17131713
when: &BinBoolExp{
1714-
op: OR,
1714+
op: Or,
17151715
left: &ColSelector{col: "is_deleted"},
17161716
right: &ColSelector{col: "is_expired"},
17171717
},
@@ -1736,7 +1736,7 @@ func TestParseExp(t *testing.T) {
17361736
whenThen: []whenThenClause{
17371737
{
17381738
when: &BinBoolExp{
1739-
op: OR,
1739+
op: Or,
17401740
left: &ColSelector{col: "is_deleted"},
17411741
right: &ColSelector{col: "is_expired"},
17421742
},
@@ -1798,7 +1798,7 @@ func TestParseExp(t *testing.T) {
17981798
},
17991799
{
18001800
when: &BinBoolExp{
1801-
op: AND,
1801+
op: And,
18021802
left: &CmpBoolExp{op: GE, left: &ColSelector{col: "stock"}, right: &Integer{10}},
18031803
right: &CmpBoolExp{op: LE, left: &ColSelector{col: "stock"}, right: &Integer{50}},
18041804
},
@@ -1898,7 +1898,7 @@ func TestMultiLineStmts(t *testing.T) {
18981898
},
18991899
ds: &tableRef{table: "table1"},
19001900
where: &BinBoolExp{
1901-
op: AND,
1901+
op: And,
19021902
left: &CmpBoolExp{
19031903
op: GE,
19041904
left: &ColSelector{
@@ -2033,7 +2033,7 @@ func TestGrantRevokeStmt(t *testing.T) {
20332033
}
20342034
}
20352035

2036-
func TestExprString(t *testing.T) {
2036+
func TestExpString(t *testing.T) {
20372037
exps := []string{
20382038
"(1 + 1) / (2 * 5 - 10) % 2",
20392039
"@param LIKE 'pattern'",
@@ -2043,6 +2043,8 @@ func TestExprString(t *testing.T) {
20432043
"CASE WHEN in_stock THEN 'In Stock' END",
20442044
"CASE WHEN 1 > 0 THEN 1 ELSE 0 END",
20452045
"CASE WHEN is_active THEN 'active' WHEN is_expired THEN 'expired' ELSE 'active' END",
2046+
"'text' LIKE 'pattern'",
2047+
"'text' NOT LIKE 'pattern'",
20462048
}
20472049

20482050
for i, e := range exps {
@@ -2056,3 +2058,46 @@ func TestExprString(t *testing.T) {
20562058
})
20572059
}
20582060
}
2061+
2062+
func TestLogicOperatorPrecedence(t *testing.T) {
2063+
type testCase struct {
2064+
input string
2065+
expected string
2066+
}
2067+
2068+
testCases := []testCase{
2069+
// simple precedence
2070+
{input: "NOT true", expected: "(NOT true)"},
2071+
{input: "true AND false OR true", expected: "((true AND false) OR true)"},
2072+
{input: "NOT true AND false", expected: "((NOT true) AND false)"},
2073+
{input: "NOT true OR false", expected: "((NOT true) OR false)"},
2074+
2075+
// parentheses override precedence
2076+
{input: "(true OR false) AND true", expected: "((true OR false) AND true)"},
2077+
2078+
// multiple NOTs
2079+
{input: "NOT NOT true AND false", expected: "((NOT (NOT true)) AND false)"},
2080+
2081+
// complex nesting
2082+
{input: "true AND (false OR (NOT false))", expected: "(true AND (false OR (NOT false)))"},
2083+
{input: "NOT (true AND false) OR true", expected: "((NOT (true AND false)) OR true)"},
2084+
2085+
// AND/OR with nested groups
2086+
{input: "(true AND false) AND (true OR false)", expected: "((true AND false) AND (true OR false))"},
2087+
{input: "(true OR false) OR (NOT (true AND false))", expected: "((true OR false) OR (NOT (true AND false)))"},
2088+
2089+
// deep nesting
2090+
{input: "(true AND (false OR (NOT true))) OR (NOT false)", expected: "((true AND (false OR (NOT true))) OR (NOT false))"},
2091+
2092+
// chain of operators
2093+
{input: "true AND false OR true AND NOT false OR true", expected: "(((true AND false) OR (true AND (NOT false))) OR true)"},
2094+
}
2095+
2096+
for _, tc := range testCases {
2097+
t.Run(tc.input, func(t *testing.T) {
2098+
e, err := ParseExpFromString(tc.input)
2099+
require.NoError(t, err)
2100+
require.Equal(t, tc.expected, e.String())
2101+
})
2102+
}
2103+
}

‎embedded/sql/sql_grammar.y‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func setResult(l yyLexer, stmts []SQLStmt) {
8686
%token <id> NPARAM
8787
%token <pparam> PPARAM
8888
%token <joinType> JOINTYPE
89-
%token <logicOp> LOP
89+
%token <logicOp> AND OR
9090
%token <cmpOp> CMPOP
9191
%token <id> IDENTIFIER
9292
%token <sqlType> TYPE
@@ -102,10 +102,14 @@ func setResult(l yyLexer, stmts []SQLStmt) {
102102

103103
%left ','
104104
%right AS
105-
%left LOP
105+
106+
%left OR
107+
%left AND
108+
106109
%right LIKE
107110
%right NOT
108-
%left CMPOP
111+
112+
%left CMPOP
109113
%left '+' '-'
110114
%left '*' '/' '%'
111115
%left '.'
@@ -1216,9 +1220,14 @@ binExp:
12161220
$$ = &NumExp{left: $1, op: MODOP, right: $3}
12171221
}
12181222
|
1219-
exp LOP exp
1223+
exp AND exp
1224+
{
1225+
$$ = &BinBoolExp{left: $1, op: And, right: $3}
1226+
}
1227+
|
1228+
exp OR exp
12201229
{
1221-
$$ = &BinBoolExp{left: $1, op: $2, right: $3}
1230+
$$ = &BinBoolExp{left: $1, op: Or, right: $3}
12221231
}
12231232
|
12241233
exp CMPOP exp

0 commit comments

Comments
 (0)