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.

Fixes apache#81.

Author: benesch

src/sqlparser.rs

@@ -173,13 +173,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() {
@@ -213,15 +210,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(_) => {
@@ -497,71 +493,80 @@ impl Parser {
         })
     }
 
+    const UNARY_NOT_PREC: u8 = 15;
+    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(20),
+                    Some(Token::SQLWord(k)) if k.keyword == "BETWEEN" => Ok(20),
+                    Some(Token::SQLWord(k)) if k.keyword == "LIKE" => Ok(20),
+                    _ => Ok(Self::UNARY_NOT_PREC),
+                },
+                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(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;
                 }
             }
         }

tests/sqlparser_common.rs

@@ -356,38 +356,98 @@ 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()))),
+            }),
         },
-        select.selection.unwrap()
     );
+
+    // 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,
+            }),
+        },
+    );
+}
+
+#[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]