Skip to content

Fix the precedence of NOT LIKE #82

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 2, 2019
Merged

Fix the precedence of NOT LIKE #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f55e3d5
Introduce a peek_nth_token method
benesch May 31, 2019
90bcf55
Fix the precedence of NOT LIKE
benesch May 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 59 additions & 54 deletions src/sqlparser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,10 @@ impl Parser {
}
"CASE" => self.parse_case_expression(),
"CAST" => self.parse_cast_expression(),
"NOT" => {
let p = self.get_precedence(&Token::make_keyword("NOT"))?;
Ok(ASTNode::SQLUnary {
operator: SQLOperator::Not,
expr: Box::new(self.parse_subexpr(p)?),
})
}
"NOT" => Ok(ASTNode::SQLUnary {
operator: SQLOperator::Not,
expr: Box::new(self.parse_subexpr(Self::UNARY_NOT_PREC)?),
}),
// Here `w` is a word, check if it's a part of a multi-part
// identifier, a function call, or a simple identifier:
_ => match self.peek_token() {
Expand Down Expand Up @@ -230,15 +227,14 @@ impl Parser {
}, // End of Token::SQLWord
Token::Mult => Ok(ASTNode::SQLWildcard),
tok @ Token::Minus | tok @ Token::Plus => {
let p = self.get_precedence(&tok)?;
let operator = if tok == Token::Plus {
SQLOperator::Plus
} else {
SQLOperator::Minus
};
Ok(ASTNode::SQLUnary {
operator,
expr: Box::new(self.parse_subexpr(p)?),
expr: Box::new(self.parse_subexpr(Self::PLUS_MINUS_PREC)?),
})
}
Token::Number(_) | Token::SingleQuotedString(_) | Token::NationalStringLiteral(_) => {
Expand Down Expand Up @@ -510,10 +506,9 @@ impl Parser {
pub fn parse_between(&mut self, expr: ASTNode, negated: bool) -> Result<ASTNode, ParserError> {
// Stop parsing subexpressions for <low> and <high> on tokens with
// precedence lower than that of `BETWEEN`, such as `AND`, `IS`, etc.
let prec = self.get_precedence(&Token::make_keyword("BETWEEN"))?;
let low = self.parse_subexpr(prec)?;
let low = self.parse_subexpr(Self::BETWEEN_PREC)?;
self.expect_keyword("AND")?;
let high = self.parse_subexpr(prec)?;
let high = self.parse_subexpr(Self::BETWEEN_PREC)?;
Ok(ASTNode::SQLBetween {
expr: Box::new(expr),
negated,
Expand All @@ -530,71 +525,81 @@ impl Parser {
})
}

const UNARY_NOT_PREC: u8 = 15;
const BETWEEN_PREC: u8 = 20;
const PLUS_MINUS_PREC: u8 = 30;

/// Get the precedence of the next token
pub fn get_next_precedence(&self) -> Result<u8, ParserError> {
if let Some(token) = self.peek_token() {
self.get_precedence(&token)
debug!("get_precedence() {:?}", token);

match &token {
Token::SQLWord(k) if k.keyword == "OR" => Ok(5),
Token::SQLWord(k) if k.keyword == "AND" => Ok(10),
Token::SQLWord(k) if k.keyword == "NOT" => match &self.peek_nth_token(1) {
// The precedence of NOT varies depending on keyword that
// follows it. If it is followed by IN, BETWEEN, or LIKE,
// it takes on the precedence of those tokens. Otherwise it
// takes on UNARY_NOT_PREC.
Some(Token::SQLWord(k)) if k.keyword == "IN" => Ok(Self::BETWEEN_PREC),
Some(Token::SQLWord(k)) if k.keyword == "BETWEEN" => Ok(Self::BETWEEN_PREC),
Some(Token::SQLWord(k)) if k.keyword == "LIKE" => Ok(Self::BETWEEN_PREC),
_ => Ok(Self::UNARY_NOT_PREC),
},
Token::SQLWord(k) if k.keyword == "IS" => Ok(17),
Token::SQLWord(k) if k.keyword == "IN" => Ok(Self::BETWEEN_PREC),
Token::SQLWord(k) if k.keyword == "BETWEEN" => Ok(Self::BETWEEN_PREC),
Token::SQLWord(k) if k.keyword == "LIKE" => Ok(Self::BETWEEN_PREC),
Token::Eq | Token::Lt | Token::LtEq | Token::Neq | Token::Gt | Token::GtEq => {
Ok(20)
}
Token::Plus | Token::Minus => Ok(Self::PLUS_MINUS_PREC),
Token::Mult | Token::Div | Token::Mod => Ok(40),
Token::DoubleColon => Ok(50),
_ => Ok(0),
}
} else {
Ok(0)
}
}

/// Get the precedence of a token
pub fn get_precedence(&self, tok: &Token) -> Result<u8, ParserError> {
debug!("get_precedence() {:?}", tok);

match tok {
Token::SQLWord(k) if k.keyword == "OR" => Ok(5),
Token::SQLWord(k) if k.keyword == "AND" => Ok(10),
Token::SQLWord(k) if k.keyword == "NOT" => Ok(15),
Token::SQLWord(k) if k.keyword == "IS" => Ok(17),
Token::SQLWord(k) if k.keyword == "IN" => Ok(20),
Token::SQLWord(k) if k.keyword == "BETWEEN" => Ok(20),
Token::SQLWord(k) if k.keyword == "LIKE" => Ok(20),
Token::Eq | Token::Lt | Token::LtEq | Token::Neq | Token::Gt | Token::GtEq => Ok(20),
Token::Plus | Token::Minus => Ok(30),
Token::Mult | Token::Div | Token::Mod => Ok(40),
Token::DoubleColon => Ok(50),
_ => Ok(0),
}
}

/// Return first non-whitespace token that has not yet been processed
pub fn peek_token(&self) -> Option<Token> {
if let Some(n) = self.til_non_whitespace() {
self.token_at(n)
} else {
None
}
self.peek_nth_token(0)
}

/// Get the next token skipping whitespace and increment the token index
pub fn next_token(&mut self) -> Option<Token> {
/// Return nth non-whitespace token that has not yet been processed
pub fn peek_nth_token(&self, mut n: usize) -> Option<Token> {
let mut index = self.index;
loop {
match self.next_token_no_skip() {
match self.token_at(index) {
Some(Token::Whitespace(_)) => {
continue;
index += 1;
}
token => {
return token;
Some(token) => {
if n == 0 {
return Some(token);
}
index += 1;
n -= 1;
}
None => {
return None;
}
}
}
}

/// get the index for non whitepsace token
fn til_non_whitespace(&self) -> Option<usize> {
let mut index = self.index;
/// Get the next token skipping whitespace and increment the token index
pub fn next_token(&mut self) -> Option<Token> {
loop {
match self.token_at(index) {
match self.next_token_no_skip() {
Some(Token::Whitespace(_)) => {
index += 1;
}
Some(_) => {
return Some(index);
continue;
}
None => {
return None;
token => {
return token;
}
}
}
Expand Down
108 changes: 84 additions & 24 deletions tests/sqlparser_common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,40 +413,100 @@ fn parse_not_precedence() {
operator: SQLOperator::Not,
..
});
}

#[test]
fn parse_like() {
let sql = "SELECT * FROM customers WHERE name LIKE '%a'";
let select = verified_only_select(sql);
// NOT has lower precedence than BETWEEN, so the following parses as NOT (1 NOT BETWEEN 1 AND 2)
let sql = "NOT 1 NOT BETWEEN 1 AND 2";
assert_eq!(
ASTNode::SQLBinaryExpr {
left: Box::new(ASTNode::SQLIdentifier("name".to_string())),
op: SQLOperator::Like,
right: Box::new(ASTNode::SQLValue(Value::SingleQuotedString(
"%a".to_string()
))),
verified_expr(sql),
SQLUnary {
operator: SQLOperator::Not,
expr: Box::new(SQLBetween {
expr: Box::new(SQLValue(Value::Long(1))),
low: Box::new(SQLValue(Value::Long(1))),
high: Box::new(SQLValue(Value::Long(2))),
negated: true,
}),
},
select.selection.unwrap()
);
}

#[test]
fn parse_not_like() {
let sql = "SELECT * FROM customers WHERE name NOT LIKE '%a'";
let select = verified_only_select(sql);
// NOT has lower precedence than LIKE, so the following parses as NOT ('a' NOT LIKE 'b')
let sql = "NOT 'a' NOT LIKE 'b'";
assert_eq!(
ASTNode::SQLBinaryExpr {
left: Box::new(ASTNode::SQLIdentifier("name".to_string())),
op: SQLOperator::NotLike,
right: Box::new(ASTNode::SQLValue(Value::SingleQuotedString(
"%a".to_string()
))),
verified_expr(sql),
SQLUnary {
operator: SQLOperator::Not,
expr: Box::new(SQLBinaryExpr {
left: Box::new(SQLValue(Value::SingleQuotedString("a".into()))),
op: SQLOperator::NotLike,
right: Box::new(SQLValue(Value::SingleQuotedString("b".into()))),
}),
},
);

// NOT has lower precedence than IN, so the following parses as NOT (a NOT IN 'a')
let sql = "NOT a NOT IN ('a')";
assert_eq!(
verified_expr(sql),
SQLUnary {
operator: SQLOperator::Not,
expr: Box::new(SQLInList {
expr: Box::new(SQLIdentifier("a".into())),
list: vec![SQLValue(Value::SingleQuotedString("a".into()))],
negated: true,
}),
},
select.selection.unwrap()
);
}

#[test]
fn parse_like() {
fn chk(negated: bool) {
let sql = &format!(
"SELECT * FROM customers WHERE name {}LIKE '%a'",
if negated { "NOT " } else { "" }
);
let select = verified_only_select(sql);
assert_eq!(
ASTNode::SQLBinaryExpr {
left: Box::new(ASTNode::SQLIdentifier("name".to_string())),
op: if negated {
SQLOperator::NotLike
} else {
SQLOperator::Like
},
right: Box::new(ASTNode::SQLValue(Value::SingleQuotedString(
"%a".to_string()
))),
},
select.selection.unwrap()
);

// This statement tests that LIKE and NOT LIKE have the same precedence.
// This was previously mishandled (#81).
let sql = &format!(
"SELECT * FROM customers WHERE name {}LIKE '%a' IS NULL",
if negated { "NOT " } else { "" }
);
let select = verified_only_select(sql);
assert_eq!(
ASTNode::SQLIsNull(Box::new(ASTNode::SQLBinaryExpr {
left: Box::new(ASTNode::SQLIdentifier("name".to_string())),
op: if negated {
SQLOperator::NotLike
} else {
SQLOperator::Like
},
right: Box::new(ASTNode::SQLValue(Value::SingleQuotedString(
"%a".to_string()
))),
})),
select.selection.unwrap()
);
}
chk(false);
chk(true);
}

#[test]
fn parse_in_list() {
fn chk(negated: bool) {
Expand Down