Skip to content

Commit

Permalink
fix(spanner/spansql): fix parsing of NOT IN operator (#3724)
Browse files Browse the repository at this point in the history
The parsing of comparison operators was erroneously capturing any NOT
token assuming it would be followed by a LIKE or BETWEEN. Instead,
change that code to explicitly look ahead for the limited combinations
allowed, and do the same for parsing IN and NOT IN.

Fixes #3715.
  • Loading branch information
dsymonds committed Feb 24, 2021
1 parent 55508ef commit 7636478
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 34 deletions.
58 changes: 24 additions & 34 deletions spanner/spansql/parser.go
Expand Up @@ -2389,15 +2389,12 @@ func (p *parser) parseInOp() (Expr, *parseError) {
return nil, err
}

// TODO: do we need to do lookahead?

inOp := InOp{LHS: expr}
if p.eat("NOT") {
if p.eat("NOT", "IN") {
inOp.Neg = true
}

if !p.eat("IN") {
// TODO: push back the "NOT"?
} else if p.eat("IN") {
// Okay.
} else {
return expr, nil
}

Expand Down Expand Up @@ -2431,37 +2428,30 @@ func (p *parser) parseComparisonOp() (Expr, *parseError) {
}

for {
tok := p.next()
if tok.err != nil {
p.back()
break
}
// There's a need for two token lookahead.
var op ComparisonOperator
var ok, rhs2 bool
if tok.value == "NOT" {
var rhs2 bool
if p.eat("NOT", "LIKE") {
op = NotLike
} else if p.eat("NOT", "BETWEEN") {
op, rhs2 = NotBetween, true
} else if p.eat("LIKE") {
op = Like
} else if p.eat("BETWEEN") {
op, rhs2 = Between, true
} else {
// Check for a symbolic operator.
tok := p.next()
switch {
case tok.err != nil:
// TODO: Does this need to push back two?
return nil, err
case tok.value == "LIKE":
op, ok = NotLike, true
case tok.value == "BETWEEN":
op, ok, rhs2 = NotBetween, true, true
default:
// TODO: Does this need to push back two?
return nil, p.errorf("got %q, want LIKE or BETWEEN", tok.value)
if tok.err != nil {
p.back()
break
}
} else if tok.value == "LIKE" {
op, ok = Like, true
} else if tok.value == "BETWEEN" {
op, ok, rhs2 = Between, true, true
} else {
var ok bool
op, ok = symbolicOperators[tok.value]
}
if !ok {
p.back()
break
if !ok {
p.back()
break
}
}

rhs, err := p.parseArithOp()
Expand Down
1 change: 1 addition & 0 deletions spanner/spansql/parser_test.go
Expand Up @@ -255,6 +255,7 @@ func TestParseExpr(t *testing.T) {
{`A AND NOT B`, LogicalOp{LHS: ID("A"), Op: And, RHS: LogicalOp{Op: Not, RHS: ID("B")}}},
{`X BETWEEN Y AND Z`, ComparisonOp{LHS: ID("X"), Op: Between, RHS: ID("Y"), RHS2: ID("Z")}},
{`@needle IN UNNEST(@haystack)`, InOp{LHS: Param("needle"), RHS: []Expr{Param("haystack")}, Unnest: true}},
{`@needle NOT IN UNNEST(@haystack)`, InOp{LHS: Param("needle"), Neg: true, RHS: []Expr{Param("haystack")}, Unnest: true}},

// String literal:
// Accept double quote and single quote.
Expand Down

0 comments on commit 7636478

Please sign in to comment.