Separate statement from expr parsing (4/?)

Continuing from https://github.com/andygrove/sqlparser-rs/pull/33#issuecomment-453060427

This stops the parser from accepting (and the AST from being able to
represent) SQL look-alike code that makes no sense, e.g.

    SELECT ... FROM (CREATE TABLE ...) foo
    SELECT ... FROM (1+CAST(...)) foo

Generally this makes the AST less "partially typed": meaning certain
parts are strongly typed (e.g. SELECT can only contain projections,
relations, etc.), while everything that didn't get its own type is
dumped into ASTNode, effectively untyped. After a few more fixes (yet
to be implemented), `ASTNode` could become an `SQLExpression`. The
Pratt-style expression parser (returning an SQLExpression) would be
invoked from the top-down parser in places where a generic expression
is expected (e.g. after SELECT <...>, WHERE <...>, etc.), while things
like select's `projection` and `relation` could be more appropriately
(narrowly) typed.


Since the diff is quite large due to necessarily large number of
mechanical changes, here's an overview:

1) Interface changes:

   - A new AST enum - `SQLStatement` - is split out of ASTNode:

     - The variants of the ASTNode enum, which _only_ make sense as a top
       level statement (INSERT, UPDATE, DELETE, CREATE, ALTER, COPY) are
       _moved_ to the new enum, with no other changes.
     - SQLSelect is _duplicated_: now available both as a variant in
       SQLStatement::SQLSelect (top-level SELECT) and ASTNode:: (subquery).

   - The main entry point (Parser::parse_sql) now expects an SQL statement
     as input, and returns an `SQLStatement`.

2) Parser changes: instead of detecting the top-level constructs deep
down in the precedence parser (`parse_prefix`) we are able to do it
just right after setting up the parser in the `parse_sql` entry point

(SELECT, again, is kept in the expression parser to demonstrate how
subqueries could be implemented).

The rest of parser changes are mechanical ASTNode -> SQLStatement
replacements resulting from the AST change.

3) Testing changes: for every test - depending on whether the input was
a complete statement or an expresssion -  I used an appropriate helper
function:

   - `verified` (parses SQL, checks that it round-trips, and returns
     the AST) - was replaced by `verified_stmt` or `verified_expr`.

   - `parse_sql` (which returned AST without checking it round-tripped)
     was replaced by:

     - `parse_sql_expr` (same function, for expressions)

     - `one_statement_parses_to` (formerly `parses_to`), extended to
       deal with statements that are not expected to round-trip.
       The weird name is to reduce further churn when implementing
       multi-statement parsing.

     - `verified_stmt` (in 4 testcases that actually round-tripped)

Author: nickolay

src/sqlast/mod.rs

@@ -79,52 +79,6 @@ pub enum ASTNode {
     },
     /// SELECT
     SQLSelect(SQLSelect),
-    /// INSERT
-    SQLInsert {
-        /// TABLE
-        table_name: String,
-        /// COLUMNS
-        columns: Vec<SQLIdent>,
-        /// VALUES (vector of rows to insert)
-        values: Vec<Vec<ASTNode>>,
-    },
-    SQLCopy {
-        /// TABLE
-        table_name: String,
-        /// COLUMNS
-        columns: Vec<SQLIdent>,
-        /// VALUES a vector of values to be copied
-        values: Vec<Option<String>>,
-    },
-    /// UPDATE
-    SQLUpdate {
-        /// TABLE
-        table_name: String,
-        /// Column assignments
-        assignments: Vec<SQLAssignment>,
-        /// WHERE
-        selection: Option<Box<ASTNode>>,
-    },
-    /// DELETE
-    SQLDelete {
-        /// FROM
-        relation: Option<Box<ASTNode>>,
-        /// WHERE
-        selection: Option<Box<ASTNode>>,
-    },
-    /// CREATE TABLE
-    SQLCreateTable {
-        /// Table name
-        name: String,
-        /// Optional schema
-        columns: Vec<SQLColumnDef>,
-    },
-    /// ALTER TABLE
-    SQLAlterTable {
-        /// Table name
-        name: String,
-        operation: AlterOperation,
-    },
 }
 
 impl ToString for ASTNode {
@@ -186,7 +140,68 @@ impl ToString for ASTNode {
                 }
             }
             ASTNode::SQLSelect(s) => s.to_string(),
-            ASTNode::SQLInsert {
+        }
+    }
+}
+
+/// A top-level statement (SELECT, INSERT, CREATE, etc.)
+#[derive(Debug, Clone, PartialEq)]
+pub enum SQLStatement {
+    /// SELECT
+    SQLSelect(SQLSelect),
+    /// INSERT
+    SQLInsert {
+        /// TABLE
+        table_name: String,
+        /// COLUMNS
+        columns: Vec<SQLIdent>,
+        /// VALUES (vector of rows to insert)
+        values: Vec<Vec<ASTNode>>,
+    },
+    SQLCopy {
+        /// TABLE
+        table_name: String,
+        /// COLUMNS
+        columns: Vec<SQLIdent>,
+        /// VALUES a vector of values to be copied
+        values: Vec<Option<String>>,
+    },
+    /// UPDATE
+    SQLUpdate {
+        /// TABLE
+        table_name: String,
+        /// Column assignments
+        assignments: Vec<SQLAssignment>,
+        /// WHERE
+        selection: Option<Box<ASTNode>>,
+    },
+    /// DELETE
+    SQLDelete {
+        /// FROM
+        relation: Option<Box<ASTNode>>,
+        /// WHERE
+        selection: Option<Box<ASTNode>>,
+    },
+    /// CREATE TABLE
+    SQLCreateTable {
+        /// Table name
+        name: String,
+        /// Optional schema
+        columns: Vec<SQLColumnDef>,
+    },
+    /// ALTER TABLE
+    SQLAlterTable {
+        /// Table name
+        name: String,
+        operation: AlterOperation,
+    },
+}
+
+impl ToString for SQLStatement {
+    fn to_string(&self) -> String {
+        match self {
+            SQLStatement::SQLSelect(s) => s.to_string(),
+            SQLStatement::SQLInsert {
                 table_name,
                 columns,
                 values,
@@ -211,7 +226,7 @@ impl ToString for ASTNode {
                 }
                 s
             }
-            ASTNode::SQLCopy {
+            SQLStatement::SQLCopy {
                 table_name,
                 columns,
                 values,
@@ -241,7 +256,7 @@ impl ToString for ASTNode {
                 s += "\n\\.";
                 s
             }
-            ASTNode::SQLUpdate {
+            SQLStatement::SQLUpdate {
                 table_name,
                 assignments,
                 selection,
@@ -262,7 +277,7 @@ impl ToString for ASTNode {
                 }
                 s
             }
-            ASTNode::SQLDelete {
+            SQLStatement::SQLDelete {
                 relation,
                 selection,
             } => {
@@ -275,7 +290,7 @@ impl ToString for ASTNode {
                 }
                 s
             }
-            ASTNode::SQLCreateTable { name, columns } => format!(
+            SQLStatement::SQLCreateTable { name, columns } => format!(
                 "CREATE TABLE {} ({})",
                 name,
                 columns
@@ -284,7 +299,7 @@ impl ToString for ASTNode {
                     .collect::<Vec<String>>()
                     .join(", ")
             ),
-            ASTNode::SQLAlterTable { name, operation } => {
+            SQLStatement::SQLAlterTable { name, operation } => {
                 format!("ALTER TABLE {} {}", name, operation.to_string())
             }
         }

src/sqlparser.rs

@@ -54,11 +54,30 @@ impl Parser {
     }
 
     /// Parse a SQL statement and produce an Abstract Syntax Tree (AST)
-    pub fn parse_sql(dialect: &Dialect, sql: String) -> Result<ASTNode, ParserError> {
+    pub fn parse_sql(dialect: &Dialect, sql: String) -> Result<SQLStatement, ParserError> {
         let mut tokenizer = Tokenizer::new(dialect, &sql);
         let tokens = tokenizer.tokenize()?;
         let mut parser = Parser::new(tokens);
-        parser.parse()
+        parser.parse_statement()
+    }
+
+    /// Parse a single top-level statement (such as SELECT, INSERT, CREATE, etc.)
+    pub fn parse_statement(&mut self) -> Result<SQLStatement, ParserError> {
+        match self.next_token() {
+            Some(t) => match t {
+                Token::SQLWord(w) => match w.keyword.as_ref() {
+                    "SELECT" => Ok(SQLStatement::SQLSelect(self.parse_select()?)),
+                    "CREATE" => Ok(self.parse_create()?),
+                    "DELETE" => Ok(self.parse_delete()?),
+                    "INSERT" => Ok(self.parse_insert()?),
+                    "ALTER" => Ok(self.parse_alter()?),
+                    "COPY" => Ok(self.parse_copy()?),
+                    unexpected => parser_err!(format!("Unexpected statement {:?}", unexpected)),
+                },
+                unexpected => parser_err!(format!("Unexpected token {:?}", unexpected)),
+            },
+            _ => parser_err!("Unexpected end of file"),
+        }
     }
 
     /// Parse a new expression
@@ -111,12 +130,7 @@ impl Parser {
         match self.next_token() {
             Some(t) => match t {
                 Token::SQLWord(w) => match w.keyword.as_ref() {
-                    "SELECT" => Ok(self.parse_select()?),
-                    "CREATE" => Ok(self.parse_create()?),
-                    "DELETE" => Ok(self.parse_delete()?),
-                    "INSERT" => Ok(self.parse_insert()?),
-                    "ALTER" => Ok(self.parse_alter()?),
-                    "COPY" => Ok(self.parse_copy()?),
+                    "SELECT" => Ok(ASTNode::SQLSelect(self.parse_select()?)),
                     "TRUE" | "FALSE" | "NULL" => {
                         self.prev_token();
                         self.parse_sql_value()
@@ -495,7 +509,7 @@ impl Parser {
     }
 
     /// Parse a SQL CREATE statement
-    pub fn parse_create(&mut self) -> Result<ASTNode, ParserError> {
+    pub fn parse_create(&mut self) -> Result<SQLStatement, ParserError> {
         if self.parse_keywords(vec!["TABLE"]) {
             let table_name = self.parse_tablename()?;
             // parse optional column list (schema)
@@ -562,7 +576,7 @@ impl Parser {
                     }
                 }
             }
-            Ok(ASTNode::SQLCreateTable {
+            Ok(SQLStatement::SQLCreateTable {
                 name: table_name,
                 columns,
             })
@@ -608,7 +622,7 @@ impl Parser {
         }
     }
 
-    pub fn parse_alter(&mut self) -> Result<ASTNode, ParserError> {
+    pub fn parse_alter(&mut self) -> Result<SQLStatement, ParserError> {
         self.expect_keyword("TABLE")?;
         let _ = self.parse_keyword("ONLY");
         let table_name = self.parse_tablename()?;
@@ -632,14 +646,14 @@ impl Parser {
                     self.peek_token()
                 ));
             };
-        Ok(ASTNode::SQLAlterTable {
+        Ok(SQLStatement::SQLAlterTable {
             name: table_name,
             operation: operation?,
         })
     }
 
     /// Parse a copy statement
-    pub fn parse_copy(&mut self) -> Result<ASTNode, ParserError> {
+    pub fn parse_copy(&mut self) -> Result<SQLStatement, ParserError> {
         let table_name = self.parse_tablename()?;
         let columns = if self.consume_token(&Token::LParen) {
             let column_names = self.parse_column_names()?;
@@ -652,7 +666,7 @@ impl Parser {
         self.expect_keyword("STDIN")?;
         self.expect_token(&Token::SemiColon)?;
         let values = self.parse_tsv()?;
-        Ok(ASTNode::SQLCopy {
+        Ok(SQLStatement::SQLCopy {
             table_name,
             columns,
             values,
@@ -1059,7 +1073,7 @@ impl Parser {
         }
     }
 
-    pub fn parse_delete(&mut self) -> Result<ASTNode, ParserError> {
+    pub fn parse_delete(&mut self) -> Result<SQLStatement, ParserError> {
         let relation: Option<Box<ASTNode>> = if self.parse_keyword("FROM") {
             Some(Box::new(self.parse_expr(0)?))
         } else {
@@ -1081,15 +1095,15 @@ impl Parser {
                 next_token
             ))
         } else {
-            Ok(ASTNode::SQLDelete {
+            Ok(SQLStatement::SQLDelete {
                 relation,
                 selection,
             })
         }
     }
 
     /// Parse a SELECT statement
-    pub fn parse_select(&mut self) -> Result<ASTNode, ParserError> {
+    pub fn parse_select(&mut self) -> Result<SQLSelect, ParserError> {
         let projection = self.parse_expr_list()?;
 
         let (relation, joins): (Option<Box<ASTNode>>, Vec<Join>) = if self.parse_keyword("FROM") {
@@ -1139,7 +1153,7 @@ impl Parser {
                 next_token
             ))
         } else {
-            Ok(ASTNode::SQLSelect(SQLSelect {
+            Ok(SQLSelect {
                 projection,
                 selection,
                 relation,
@@ -1148,7 +1162,7 @@ impl Parser {
                 order_by,
                 group_by,
                 having,
-            }))
+            })
         }
     }
 
@@ -1287,7 +1301,7 @@ impl Parser {
     }
 
     /// Parse an INSERT statement
-    pub fn parse_insert(&mut self) -> Result<ASTNode, ParserError> {
+    pub fn parse_insert(&mut self) -> Result<SQLStatement, ParserError> {
         self.expect_keyword("INTO")?;
         let table_name = self.parse_tablename()?;
         let columns = if self.consume_token(&Token::LParen) {
@@ -1301,7 +1315,7 @@ impl Parser {
         self.expect_token(&Token::LParen)?;
         let values = self.parse_expr_list()?;
         self.expect_token(&Token::RParen)?;
-        Ok(ASTNode::SQLInsert {
+        Ok(SQLStatement::SQLInsert {
             table_name,
             columns,
             values: vec![values],

tests/sqlparser_ansi.rs

@@ -9,7 +9,7 @@ use sqlparser::sqltokenizer::*;
 #[test]
 fn parse_simple_select() {
     let sql = String::from("SELECT id, fname, lname FROM customer WHERE id = 1");
-    let ast = parse_sql(&sql);
+    let ast = parse_sql_expr(&sql);
     match ast {
         ASTNode::SQLSelect(SQLSelect { projection, .. }) => {
             assert_eq!(3, projection.len());
@@ -18,7 +18,7 @@ fn parse_simple_select() {
     }
 }
 
-fn parse_sql(sql: &str) -> ASTNode {
+fn parse_sql_expr(sql: &str) -> ASTNode {
     let dialect = AnsiSqlDialect {};
     let mut tokenizer = Tokenizer::new(&dialect, &sql);
     let tokens = tokenizer.tokenize().unwrap();