Merge pull request #115 from nickolay/pr/followups

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
+    /// Unqualified wildcard (`*`). SQL allows this in limited contexts, such as:
-    /// after `SELECT` or as part of an aggregate function, e.g. `COUNT(*)`, but we
+    /// - right after `SELECT` (which is represented as a [SQLSelectItem::Wildcard] instead)
-    /// currently accept it in contexts where it doesn't make sense, such as `* + *`
+    /// - 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
+    /// `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
+    /// Note we only recognize a complete single expression as `<condition>`,
-    /// https://jakewheat.github.io/sql-overview/sql-2011-foundation-grammar.html#simple-when-clause
+    /// 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/table_key.rs

@@ -1,73 +0,0 @@
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-use super::{SQLIdent, SQLObjectName};
-
-#[derive(Debug, Clone, PartialEq)]
-pub enum AlterOperation {
-    AddConstraint(TableKey),
-    RemoveConstraint { name: SQLIdent },
-}
-
-impl ToString for AlterOperation {
-    fn to_string(&self) -> String {
-        match self {
-            AlterOperation::AddConstraint(table_key) => {
-                format!("ADD CONSTRAINT {}", table_key.to_string())
-            }
-            AlterOperation::RemoveConstraint { name } => format!("REMOVE CONSTRAINT {}", name),
-        }
-    }
-}
-
-#[derive(Debug, Clone, PartialEq)]
-pub struct Key {
-    pub name: SQLIdent,
-    pub columns: Vec<SQLIdent>,
-}
-
-#[derive(Debug, Clone, PartialEq)]
-pub enum TableKey {
-    PrimaryKey(Key),
-    UniqueKey(Key),
-    Key(Key),
-    ForeignKey {
-        key: Key,
-        foreign_table: SQLObjectName,
-        referred_columns: Vec<SQLIdent>,
-    },
-}
-
-impl ToString for TableKey {
-    fn to_string(&self) -> String {
-        match self {
-            TableKey::PrimaryKey(ref key) => {
-                format!("{} PRIMARY KEY ({})", key.name, key.columns.join(", "))
-            }
-            TableKey::UniqueKey(ref key) => {
-                format!("{} UNIQUE KEY ({})", key.name, key.columns.join(", "))
-            }
-            TableKey::Key(ref key) => format!("{} KEY ({})", key.name, key.columns.join(", ")),
-            TableKey::ForeignKey {
-                key,
-                foreign_table,
-                referred_columns,
-            } => format!(
-                "{} FOREIGN KEY ({}) REFERENCES {}({})",
-                key.name,
-                key.columns.join(", "),
-                foreign_table.to_string(),
-                referred_columns.join(", ")
-            ),
-        }
-    }
-}

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

@@ -896,10 +896,7 @@ impl Parser {
         } else if self.parse_keyword("VIEW") {
             SQLObjectType::View
         } else {
-            return parser_err!(format!(
+            return self.expected("TABLE or VIEW after DROP", self.peek_token());
-                "Unexpected token after DROP: {:?}",
-                self.peek_token()
-            ));
         };
         let if_exists = self.parse_keywords(vec!["IF", "EXISTS"]);
         let mut names = vec![];
@@ -1021,10 +1018,7 @@ impl Parser {
             self.expect_token(&Token::RParen)?;
             ColumnOption::Check(expr)
         } else {
-            return parser_err!(format!(
+            return self.expected("column option", self.peek_token());
-                "Unexpected token in column definition: {:?}",
-                self.peek_token()
-            ));
         };
 
         Ok(ColumnOptionDef { name, option })
@@ -1218,21 +1212,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),
         }
     }
 
@@ -1623,16 +1607,13 @@ impl Parser {
         let relation = self.parse_table_factor()?;
         let mut joins = vec![];
         loop {
-            let join = match &self.peek_token() {
+            let join = if self.parse_keyword("CROSS") {
-                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,
                 }
-                }
+            } else {
-                _ => {
                 let natural = self.parse_keyword("NATURAL");
                 let peek_keyword = if let Some(Token::SQLWord(kw)) = self.peek_token() {
                     kw.keyword
@@ -1668,7 +1649,6 @@ impl Parser {
                     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]
@@ -1904,6 +1914,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]