Skip to content

Commit 90bcf55

Browse files
committed
Fix the precedence of NOT LIKE
NOT LIKE has the same precedence as the LIKE operator. The parser was previously assigning it the precedence of the unary NOT operator. NOT BETWEEN and NOT IN are treated similarly, as they are equivalent, from a precedence perspective, to NOT LIKE. The fix for this requires associating precedences with sequences of tokens, rather than single tokens, so that "NOT LIKE" and "NOT <expr>" can have different preferences. Perhaps surprisingly, this change is not very invasive. An alternative I considered involved adjusting the tokenizer to lex NOT, NOT LIKE, NOT BETWEEN, and NOT IN as separate tokens. This broke symmetry in strange ways, though, as NotLike, NotBetween, and NotIn gained dedicated tokens, while LIKE, BETWEEN, and IN remained as stringly identifiers. Fixes #81.
1 parent f55e3d5 commit 90bcf55

File tree

2 files changed

+122
-57
lines changed

2 files changed

+122
-57
lines changed

src/sqlparser.rs

+38-33
Original file line numberDiff line numberDiff line change
@@ -190,13 +190,10 @@ impl Parser {
190190
}
191191
"CASE" => self.parse_case_expression(),
192192
"CAST" => self.parse_cast_expression(),
193-
"NOT" => {
194-
let p = self.get_precedence(&Token::make_keyword("NOT"))?;
195-
Ok(ASTNode::SQLUnary {
196-
operator: SQLOperator::Not,
197-
expr: Box::new(self.parse_subexpr(p)?),
198-
})
199-
}
193+
"NOT" => Ok(ASTNode::SQLUnary {
194+
operator: SQLOperator::Not,
195+
expr: Box::new(self.parse_subexpr(Self::UNARY_NOT_PREC)?),
196+
}),
200197
// Here `w` is a word, check if it's a part of a multi-part
201198
// identifier, a function call, or a simple identifier:
202199
_ => match self.peek_token() {
@@ -230,15 +227,14 @@ impl Parser {
230227
}, // End of Token::SQLWord
231228
Token::Mult => Ok(ASTNode::SQLWildcard),
232229
tok @ Token::Minus | tok @ Token::Plus => {
233-
let p = self.get_precedence(&tok)?;
234230
let operator = if tok == Token::Plus {
235231
SQLOperator::Plus
236232
} else {
237233
SQLOperator::Minus
238234
};
239235
Ok(ASTNode::SQLUnary {
240236
operator,
241-
expr: Box::new(self.parse_subexpr(p)?),
237+
expr: Box::new(self.parse_subexpr(Self::PLUS_MINUS_PREC)?),
242238
})
243239
}
244240
Token::Number(_) | Token::SingleQuotedString(_) | Token::NationalStringLiteral(_) => {
@@ -510,10 +506,9 @@ impl Parser {
510506
pub fn parse_between(&mut self, expr: ASTNode, negated: bool) -> Result<ASTNode, ParserError> {
511507
// Stop parsing subexpressions for <low> and <high> on tokens with
512508
// precedence lower than that of `BETWEEN`, such as `AND`, `IS`, etc.
513-
let prec = self.get_precedence(&Token::make_keyword("BETWEEN"))?;
514-
let low = self.parse_subexpr(prec)?;
509+
let low = self.parse_subexpr(Self::BETWEEN_PREC)?;
515510
self.expect_keyword("AND")?;
516-
let high = self.parse_subexpr(prec)?;
511+
let high = self.parse_subexpr(Self::BETWEEN_PREC)?;
517512
Ok(ASTNode::SQLBetween {
518513
expr: Box::new(expr),
519514
negated,
@@ -530,35 +525,45 @@ impl Parser {
530525
})
531526
}
532527

528+
const UNARY_NOT_PREC: u8 = 15;
529+
const BETWEEN_PREC: u8 = 20;
530+
const PLUS_MINUS_PREC: u8 = 30;
531+
533532
/// Get the precedence of the next token
534533
pub fn get_next_precedence(&self) -> Result<u8, ParserError> {
535534
if let Some(token) = self.peek_token() {
536-
self.get_precedence(&token)
535+
debug!("get_precedence() {:?}", token);
536+
537+
match &token {
538+
Token::SQLWord(k) if k.keyword == "OR" => Ok(5),
539+
Token::SQLWord(k) if k.keyword == "AND" => Ok(10),
540+
Token::SQLWord(k) if k.keyword == "NOT" => match &self.peek_nth_token(1) {
541+
// The precedence of NOT varies depending on keyword that
542+
// follows it. If it is followed by IN, BETWEEN, or LIKE,
543+
// it takes on the precedence of those tokens. Otherwise it
544+
// takes on UNARY_NOT_PREC.
545+
Some(Token::SQLWord(k)) if k.keyword == "IN" => Ok(Self::BETWEEN_PREC),
546+
Some(Token::SQLWord(k)) if k.keyword == "BETWEEN" => Ok(Self::BETWEEN_PREC),
547+
Some(Token::SQLWord(k)) if k.keyword == "LIKE" => Ok(Self::BETWEEN_PREC),
548+
_ => Ok(Self::UNARY_NOT_PREC),
549+
},
550+
Token::SQLWord(k) if k.keyword == "IS" => Ok(17),
551+
Token::SQLWord(k) if k.keyword == "IN" => Ok(Self::BETWEEN_PREC),
552+
Token::SQLWord(k) if k.keyword == "BETWEEN" => Ok(Self::BETWEEN_PREC),
553+
Token::SQLWord(k) if k.keyword == "LIKE" => Ok(Self::BETWEEN_PREC),
554+
Token::Eq | Token::Lt | Token::LtEq | Token::Neq | Token::Gt | Token::GtEq => {
555+
Ok(20)
556+
}
557+
Token::Plus | Token::Minus => Ok(Self::PLUS_MINUS_PREC),
558+
Token::Mult | Token::Div | Token::Mod => Ok(40),
559+
Token::DoubleColon => Ok(50),
560+
_ => Ok(0),
561+
}
537562
} else {
538563
Ok(0)
539564
}
540565
}
541566

542-
/// Get the precedence of a token
543-
pub fn get_precedence(&self, tok: &Token) -> Result<u8, ParserError> {
544-
debug!("get_precedence() {:?}", tok);
545-
546-
match tok {
547-
Token::SQLWord(k) if k.keyword == "OR" => Ok(5),
548-
Token::SQLWord(k) if k.keyword == "AND" => Ok(10),
549-
Token::SQLWord(k) if k.keyword == "NOT" => Ok(15),
550-
Token::SQLWord(k) if k.keyword == "IS" => Ok(17),
551-
Token::SQLWord(k) if k.keyword == "IN" => Ok(20),
552-
Token::SQLWord(k) if k.keyword == "BETWEEN" => Ok(20),
553-
Token::SQLWord(k) if k.keyword == "LIKE" => Ok(20),
554-
Token::Eq | Token::Lt | Token::LtEq | Token::Neq | Token::Gt | Token::GtEq => Ok(20),
555-
Token::Plus | Token::Minus => Ok(30),
556-
Token::Mult | Token::Div | Token::Mod => Ok(40),
557-
Token::DoubleColon => Ok(50),
558-
_ => Ok(0),
559-
}
560-
}
561-
562567
/// Return first non-whitespace token that has not yet been processed
563568
pub fn peek_token(&self) -> Option<Token> {
564569
self.peek_nth_token(0)

tests/sqlparser_common.rs

+84-24
Original file line numberDiff line numberDiff line change
@@ -413,40 +413,100 @@ fn parse_not_precedence() {
413413
operator: SQLOperator::Not,
414414
..
415415
});
416-
}
417416

418-
#[test]
419-
fn parse_like() {
420-
let sql = "SELECT * FROM customers WHERE name LIKE '%a'";
421-
let select = verified_only_select(sql);
417+
// NOT has lower precedence than BETWEEN, so the following parses as NOT (1 NOT BETWEEN 1 AND 2)
418+
let sql = "NOT 1 NOT BETWEEN 1 AND 2";
422419
assert_eq!(
423-
ASTNode::SQLBinaryExpr {
424-
left: Box::new(ASTNode::SQLIdentifier("name".to_string())),
425-
op: SQLOperator::Like,
426-
right: Box::new(ASTNode::SQLValue(Value::SingleQuotedString(
427-
"%a".to_string()
428-
))),
420+
verified_expr(sql),
421+
SQLUnary {
422+
operator: SQLOperator::Not,
423+
expr: Box::new(SQLBetween {
424+
expr: Box::new(SQLValue(Value::Long(1))),
425+
low: Box::new(SQLValue(Value::Long(1))),
426+
high: Box::new(SQLValue(Value::Long(2))),
427+
negated: true,
428+
}),
429429
},
430-
select.selection.unwrap()
431430
);
432-
}
433431

434-
#[test]
435-
fn parse_not_like() {
436-
let sql = "SELECT * FROM customers WHERE name NOT LIKE '%a'";
437-
let select = verified_only_select(sql);
432+
// NOT has lower precedence than LIKE, so the following parses as NOT ('a' NOT LIKE 'b')
433+
let sql = "NOT 'a' NOT LIKE 'b'";
438434
assert_eq!(
439-
ASTNode::SQLBinaryExpr {
440-
left: Box::new(ASTNode::SQLIdentifier("name".to_string())),
441-
op: SQLOperator::NotLike,
442-
right: Box::new(ASTNode::SQLValue(Value::SingleQuotedString(
443-
"%a".to_string()
444-
))),
435+
verified_expr(sql),
436+
SQLUnary {
437+
operator: SQLOperator::Not,
438+
expr: Box::new(SQLBinaryExpr {
439+
left: Box::new(SQLValue(Value::SingleQuotedString("a".into()))),
440+
op: SQLOperator::NotLike,
441+
right: Box::new(SQLValue(Value::SingleQuotedString("b".into()))),
442+
}),
443+
},
444+
);
445+
446+
// NOT has lower precedence than IN, so the following parses as NOT (a NOT IN 'a')
447+
let sql = "NOT a NOT IN ('a')";
448+
assert_eq!(
449+
verified_expr(sql),
450+
SQLUnary {
451+
operator: SQLOperator::Not,
452+
expr: Box::new(SQLInList {
453+
expr: Box::new(SQLIdentifier("a".into())),
454+
list: vec![SQLValue(Value::SingleQuotedString("a".into()))],
455+
negated: true,
456+
}),
445457
},
446-
select.selection.unwrap()
447458
);
448459
}
449460

461+
#[test]
462+
fn parse_like() {
463+
fn chk(negated: bool) {
464+
let sql = &format!(
465+
"SELECT * FROM customers WHERE name {}LIKE '%a'",
466+
if negated { "NOT " } else { "" }
467+
);
468+
let select = verified_only_select(sql);
469+
assert_eq!(
470+
ASTNode::SQLBinaryExpr {
471+
left: Box::new(ASTNode::SQLIdentifier("name".to_string())),
472+
op: if negated {
473+
SQLOperator::NotLike
474+
} else {
475+
SQLOperator::Like
476+
},
477+
right: Box::new(ASTNode::SQLValue(Value::SingleQuotedString(
478+
"%a".to_string()
479+
))),
480+
},
481+
select.selection.unwrap()
482+
);
483+
484+
// This statement tests that LIKE and NOT LIKE have the same precedence.
485+
// This was previously mishandled (#81).
486+
let sql = &format!(
487+
"SELECT * FROM customers WHERE name {}LIKE '%a' IS NULL",
488+
if negated { "NOT " } else { "" }
489+
);
490+
let select = verified_only_select(sql);
491+
assert_eq!(
492+
ASTNode::SQLIsNull(Box::new(ASTNode::SQLBinaryExpr {
493+
left: Box::new(ASTNode::SQLIdentifier("name".to_string())),
494+
op: if negated {
495+
SQLOperator::NotLike
496+
} else {
497+
SQLOperator::Like
498+
},
499+
right: Box::new(ASTNode::SQLValue(Value::SingleQuotedString(
500+
"%a".to_string()
501+
))),
502+
})),
503+
select.selection.unwrap()
504+
);
505+
}
506+
chk(false);
507+
chk(true);
508+
}
509+
450510
#[test]
451511
fn parse_in_list() {
452512
fn chk(negated: bool) {

0 commit comments

Comments
 (0)