Doc improvements and follow-ups to the recent PRs

src/sqlast/mod.rs

@@ -56,9 +56,12 @@ pub type SQLIdent = String;
 pub enum ASTNode {
     /// Identifier e.g. table name or column name
     SQLIdentifier(SQLIdent),
-    /// Unqualified wildcard (`*`). SQL allows this in limited contexts (such as right
-    /// after `SELECT` or as part of an aggregate function, e.g. `COUNT(*)`, but we
-    /// currently accept it in contexts where it doesn't make sense, such as `* + *`
+    /// Unqualified wildcard (`*`). SQL allows this in limited contexts, such as:
+    /// - right after `SELECT` (which is represented as a [SQLSelectItem::Wildcard] instead)
+    /// - or as part of an aggregate function, e.g. `COUNT(*)`,
+    ///
+    /// ...but we currently also accept it in contexts where it doesn't make
+    /// sense, such as `* + *`
     SQLWildcard,
     /// Qualified wildcard, e.g. `alias.*` or `schema.table.*`.
     /// (Same caveats apply to SQLQualifiedWildcard as to SQLWildcard.)
@@ -119,10 +122,11 @@ pub enum ASTNode {
     SQLValue(Value),
     /// Scalar function call e.g. `LEFT(foo, 5)`
     SQLFunction(SQLFunction),
-    /// CASE [<operand>] WHEN <condition> THEN <result> ... [ELSE <result>] END
-    /// Note we only recognize a complete single expression as <condition>, not
-    /// `< 0` nor `1, 2, 3` as allowed in a <simple when clause> per
-    /// https://jakewheat.github.io/sql-overview/sql-2011-foundation-grammar.html#simple-when-clause
+    /// `CASE [<operand>] WHEN <condition> THEN <result> ... [ELSE <result>] END`
+    ///
+    /// Note we only recognize a complete single expression as `<condition>`,
+    /// not `< 0` nor `1, 2, 3` as allowed in a `<simple when clause>` per
+    /// <https://jakewheat.github.io/sql-overview/sql-2011-foundation-grammar.html#simple-when-clause>
     SQLCase {
         operand: Option<Box<ASTNode>>,
         conditions: Vec<ASTNode>,
@@ -413,13 +417,13 @@ pub enum SQLStatement {
         names: Vec<SQLObjectName>,
         cascade: bool,
     },
-    /// { BEGIN [ TRANSACTION | WORK ] | START TRANSACTION } ...
+    /// `{ BEGIN [ TRANSACTION | WORK ] | START TRANSACTION } ...`
     SQLStartTransaction { modes: Vec<TransactionMode> },
-    /// SET TRANSACTION ...
+    /// `SET TRANSACTION ...`
     SQLSetTransaction { modes: Vec<TransactionMode> },
-    /// COMMIT [ TRANSACTION | WORK ] [ AND [ NO ] CHAIN ]
+    /// `COMMIT [ TRANSACTION | WORK ] [ AND [ NO ] CHAIN ]`
     SQLCommit { chain: bool },
-    /// ROLLBACK [ TRANSACTION | WORK ] [ AND [ NO ] CHAIN ]
+    /// `ROLLBACK [ TRANSACTION | WORK ] [ AND [ NO ] CHAIN ]`
     SQLRollback { chain: bool },
 }
 

src/sqlast/query.rs

@@ -22,11 +22,11 @@ pub struct SQLQuery {
     pub body: SQLSetExpr,
     /// ORDER BY
     pub order_by: Vec<SQLOrderByExpr>,
-    /// LIMIT { <N> | ALL }
+    /// `LIMIT { <N> | ALL }`
     pub limit: Option<ASTNode>,
-    /// OFFSET <N> { ROW | ROWS }
+    /// `OFFSET <N> { ROW | ROWS }`
     pub offset: Option<ASTNode>,
-    /// FETCH { FIRST | NEXT } <N> [ PERCENT ] { ROW | ROWS } | { ONLY | WITH TIES }
+    /// `FETCH { FIRST | NEXT } <N> [ PERCENT ] { ROW | ROWS } | { ONLY | WITH TIES }`
     pub fetch: Option<Fetch>,
 }
 

src/sqlast/value.rs

@@ -27,21 +27,32 @@ pub enum Value {
     HexStringLiteral(String),
     /// Boolean value true or false
     Boolean(bool),
-    /// Date literals
+    /// `DATE '...'` literals
     Date(String),
-    /// Time literals
+    /// `TIME '...'` literals
     Time(String),
-    /// Timestamp literals, which include both a date and time
+    /// `TIMESTAMP '...'` literals
     Timestamp(String),
-    /// INTERVAL literals, e.g. INTERVAL '12:34.56' MINUTE TO SECOND (2)
+    /// INTERVAL literals, roughly in the following format:
+    /// `INTERVAL '<value>' <leading_field> [ (<leading_precision>) ]
+    /// [ TO <last_field> [ (<fractional_seconds_precision>) ] ]`,
+    /// e.g. `INTERVAL '123:45.67' MINUTE(3) TO SECOND(2)`.
+    ///
+    /// The parser does not validate the `<value>`, nor does it ensure
+    /// that the `<leading_field>` units >= the units in `<last_field>`,
+    /// so the user will have to reject intervals like `HOUR TO YEAR`.
     Interval {
         value: String,
         leading_field: SQLDateTimeField,
         leading_precision: Option<u64>,
         last_field: Option<SQLDateTimeField>,
+        /// The seconds precision can be specified in SQL source as
+        /// `INTERVAL '__' SECOND(_, x)` (in which case the `leading_field`
+        /// will be `Second` and the `last_field` will be `None`),
+        /// or as `__ TO SECOND(x)`.
         fractional_seconds_precision: Option<u64>,
     },
-    /// NULL value in insert statements,
+    /// `NULL` value
     Null,
 }
 

src/sqlparser.rs

@@ -890,10 +890,7 @@ impl Parser {
         } else if self.parse_keyword("VIEW") {
             SQLObjectType::View
         } else {
-            return parser_err!(format!(
-                "Unexpected token after DROP: {:?}",
-                self.peek_token()
-            ));
+            return self.expected("TABLE or VIEW after DROP", self.peek_token());
         };
         let if_exists = self.parse_keywords(vec!["IF", "EXISTS"]);
         let mut names = vec![];
@@ -1015,10 +1012,7 @@ impl Parser {
             self.expect_token(&Token::RParen)?;
             ColumnOption::Check(expr)
         } else {
-            return parser_err!(format!(
-                "Unexpected token in column definition: {:?}",
-                self.peek_token()
-            ));
+            return self.expected("column option", self.peek_token());
         };
 
         Ok(ColumnOptionDef { name, option })
@@ -1212,21 +1206,11 @@ impl Parser {
         }
     }
 
-    /// Parse a literal double
-    pub fn parse_literal_double(&mut self) -> Result<f64, ParserError> {
-        match self.next_token() {
-            Some(Token::Number(s)) => s.parse::<f64>().map_err(|e| {
-                ParserError::ParserError(format!("Could not parse '{}' as f64: {}", s, e))
-            }),
-            other => parser_err!(format!("Expected literal number, found {:?}", other)),
-        }
-    }
-
     /// Parse a literal string
     pub fn parse_literal_string(&mut self) -> Result<String, ParserError> {
         match self.next_token() {
             Some(Token::SingleQuotedString(ref s)) => Ok(s.clone()),
-            other => parser_err!(format!("Expected literal string, found {:?}", other)),
+            other => self.expected("literal string", other),
         }
     }
 
@@ -1614,51 +1598,47 @@ impl Parser {
         let relation = self.parse_table_factor()?;
         let mut joins = vec![];
         loop {
-            let join = match &self.peek_token() {
-                Some(Token::SQLWord(kw)) if kw.keyword == "CROSS" => {
-                    self.next_token();
-                    self.expect_keyword("JOIN")?;
-                    Join {
-                        relation: self.parse_table_factor()?,
-                        join_operator: JoinOperator::Cross,
-                    }
+            let join = if self.parse_keyword("CROSS") {
+                self.expect_keyword("JOIN")?;
+                Join {
+                    relation: self.parse_table_factor()?,
+                    join_operator: JoinOperator::Cross,
                 }
-                _ => {
-                    let natural = self.parse_keyword("NATURAL");
-                    let peek_keyword = if let Some(Token::SQLWord(kw)) = self.peek_token() {
-                        kw.keyword
-                    } else {
-                        String::default()
-                    };
+            } else {
+                let natural = self.parse_keyword("NATURAL");
+                let peek_keyword = if let Some(Token::SQLWord(kw)) = self.peek_token() {
+                    kw.keyword
+                } else {
+                    String::default()
+                };
 
-                    let join_operator_type = match peek_keyword.as_ref() {
-                        "INNER" | "JOIN" => {
-                            let _ = self.parse_keyword("INNER");
-                            self.expect_keyword("JOIN")?;
-                            JoinOperator::Inner
-                        }
-                        kw @ "LEFT" | kw @ "RIGHT" | kw @ "FULL" => {
-                            let _ = self.next_token();
-                            let _ = self.parse_keyword("OUTER");
-                            self.expect_keyword("JOIN")?;
-                            match kw {
-                                "LEFT" => JoinOperator::LeftOuter,
-                                "RIGHT" => JoinOperator::RightOuter,
-                                "FULL" => JoinOperator::FullOuter,
-                                _ => unreachable!(),
-                            }
-                        }
-                        _ if natural => {
-                            return self.expected("a join type after NATURAL", self.peek_token());
+                let join_operator_type = match peek_keyword.as_ref() {
+                    "INNER" | "JOIN" => {
+                        let _ = self.parse_keyword("INNER");
+                        self.expect_keyword("JOIN")?;
+                        JoinOperator::Inner
+                    }
+                    kw @ "LEFT" | kw @ "RIGHT" | kw @ "FULL" => {
+                        let _ = self.next_token();
+                        let _ = self.parse_keyword("OUTER");
+                        self.expect_keyword("JOIN")?;
+                        match kw {
+                            "LEFT" => JoinOperator::LeftOuter,
+                            "RIGHT" => JoinOperator::RightOuter,
+                            "FULL" => JoinOperator::FullOuter,
+                            _ => unreachable!(),
                         }
-                        _ => break,
-                    };
-                    let relation = self.parse_table_factor()?;
-                    let join_constraint = self.parse_join_constraint(natural)?;
-                    Join {
-                        relation,
-                        join_operator: join_operator_type(join_constraint),
                     }
+                    _ if natural => {
+                        return self.expected("a join type after NATURAL", self.peek_token());
+                    }
+                    _ => break,
+                };
+                let relation = self.parse_table_factor()?;
+                let join_constraint = self.parse_join_constraint(natural)?;
+                Join {
+                    relation,
+                    join_operator: join_operator_type(join_constraint),
                 }
             };
             joins.push(join);

tests/sqlparser_common.rs

@@ -401,6 +401,16 @@ fn parse_projection_nested_type() {
     //TODO: add assertions
 }
 
+#[test]
+fn parse_null_in_select() {
+    let sql = "SELECT NULL";
+    let select = verified_only_select(sql);
+    assert_eq!(
+        &ASTNode::SQLValue(Value::Null),
+        expr_from_projection(only(&select.projection)),
+    );
+}
+
 #[test]
 fn parse_escaped_single_quote_string_predicate() {
     use self::ASTNode::*;
@@ -949,7 +959,7 @@ fn parse_create_table() {
     assert!(res
         .unwrap_err()
         .to_string()
-        .contains("Unexpected token in column definition"));
+        .contains("Expected column option, found: GARBAGE"));
 }
 
 #[test]
@@ -1866,6 +1876,7 @@ fn parse_union() {
     verified_stmt("SELECT 1 UNION (SELECT 2 ORDER BY 1 LIMIT 1)");
     verified_stmt("SELECT 1 UNION SELECT 2 INTERSECT SELECT 3"); // Union[1, Intersect[2,3]]
     verified_stmt("SELECT foo FROM tab UNION SELECT bar FROM TAB");
+    verified_stmt("(SELECT * FROM new EXCEPT SELECT * FROM old) UNION ALL (SELECT * FROM old EXCEPT SELECT * FROM new) ORDER BY 1");
 }
 
 #[test]