Don't fail parsing a column definition with unexpected tokens

Since PR apache#93
`parse_column_def` parses a set of column options in a loop, e.g. given:

```
                  _______ column_def _______
CREATE TABLE foo (bar INT NOT NULL DEFAULT 1, )
                          -------- ---------
                          option 1  option 2
````

it parses column options until it encounters one of the delimiter tokens

First when we only supported `CREATE TABLE`, the set of delimiters that
stopped the parsing used to be `Token::Comma | Token::RParen`.

Then we added support for `ALTER TABLE ADD COLUMN <column_def>`. Turns
out the parser started to bail if the statement ended with a semicolon,
while attempting to parse the semicolon as a column option, as we forgot
to add it to the set of delimiter tokens.

This was recently fixed in apache#246
by including Token::SemiColon to the list, but it felt wrong to have 
to update this list, and to have a common list of delimiters for two
different contexts (CREATE TABLE with parens vs ALTER TABLE ADD COLUMN
without parens).

Also our current approach cannot handle multiple statements NOT
separated by a semicolon, as is common in MS SQL DDL. We don't
explicitly support it in `parse_statements`, but that's a use-case
like to keep in mind nevertheless.

Author: nickolay

src/parser.rs

@@ -1225,10 +1225,18 @@ impl Parser {
         };
         let mut options = vec![];
         loop {
-            match self.peek_token() {
-                Token::EOF | Token::Comma | Token::RParen | Token::SemiColon => break,
-                _ => options.push(self.parse_column_option_def()?),
-            }
+            if self.parse_keyword(Keyword::CONSTRAINT) {
+                let name = Some(self.parse_identifier()?);
+                if let Some(option) = self.parse_optional_column_option()? {
+                    options.push(ColumnOptionDef { name, option });
+                } else {
+                    return self.expected("constraint details after CONSTRAINT <name>", self.peek_token());
+                }
+            } else if let Some(option) = self.parse_optional_column_option()? {
+                options.push(ColumnOptionDef { name: None, option });
+            } else {
+                break;
+            };
         }
         Ok(ColumnDef {
             name,
@@ -1238,23 +1246,17 @@ impl Parser {
         })
     }
 
-    pub fn parse_column_option_def(&mut self) -> Result<ColumnOptionDef, ParserError> {
-        let name = if self.parse_keyword(Keyword::CONSTRAINT) {
-            Some(self.parse_identifier()?)
-        } else {
-            None
-        };
-
-        let option = if self.parse_keywords(&[Keyword::NOT, Keyword::NULL]) {
-            ColumnOption::NotNull
+    pub fn parse_optional_column_option(&mut self) -> Result<Option<ColumnOption>, ParserError> {
+        if self.parse_keywords(&[Keyword::NOT, Keyword::NULL]) {
+            Ok(Some(ColumnOption::NotNull))
         } else if self.parse_keyword(Keyword::NULL) {
-            ColumnOption::Null
+            Ok(Some(ColumnOption::Null))
         } else if self.parse_keyword(Keyword::DEFAULT) {
-            ColumnOption::Default(self.parse_expr()?)
+            Ok(Some(ColumnOption::Default(self.parse_expr()?)))
         } else if self.parse_keywords(&[Keyword::PRIMARY, Keyword::KEY]) {
-            ColumnOption::Unique { is_primary: true }
+            Ok(Some(ColumnOption::Unique { is_primary: true }))
         } else if self.parse_keyword(Keyword::UNIQUE) {
-            ColumnOption::Unique { is_primary: false }
+            Ok(Some(ColumnOption::Unique { is_primary: false }))
         } else if self.parse_keyword(Keyword::REFERENCES) {
             let foreign_table = self.parse_object_name()?;
             // PostgreSQL allows omitting the column list and
@@ -1273,28 +1275,30 @@ impl Parser {
                     break;
                 }
             }
-            ColumnOption::ForeignKey {
+            Ok(Some(ColumnOption::ForeignKey {
                 foreign_table,
                 referred_columns,
                 on_delete,
                 on_update,
-            }
+            }))
         } else if self.parse_keyword(Keyword::CHECK) {
             self.expect_token(&Token::LParen)?;
             let expr = self.parse_expr()?;
             self.expect_token(&Token::RParen)?;
-            ColumnOption::Check(expr)
+            Ok(Some(ColumnOption::Check(expr)))
         } else if self.parse_keyword(Keyword::AUTO_INCREMENT) {
             // Support AUTO_INCREMENT for MySQL
-            ColumnOption::DialectSpecific(vec![Token::make_keyword("AUTO_INCREMENT")])
+            Ok(Some(ColumnOption::DialectSpecific(vec![
+                Token::make_keyword("AUTO_INCREMENT"),
+            ])))
         } else if self.parse_keyword(Keyword::AUTOINCREMENT) {
             // Support AUTOINCREMENT for SQLite
-            ColumnOption::DialectSpecific(vec![Token::make_keyword("AUTOINCREMENT")])
+            Ok(Some(ColumnOption::DialectSpecific(vec![
+                Token::make_keyword("AUTOINCREMENT"),
+            ])))
         } else {
-            return self.expected("column option", self.peek_token());
-        };
-
-        Ok(ColumnOptionDef { name, option })
+            Ok(None)
+        }
     }
 
     pub fn parse_referential_action(&mut self) -> Result<ReferentialAction, ParserError> {

tests/sqlparser_common.rs

@@ -1141,7 +1141,14 @@ fn parse_create_table() {
     assert!(res
         .unwrap_err()
         .to_string()
-        .contains("Expected column option, found: GARBAGE"));
+        .contains("Expected \',\' or \')\' after column definition, found: GARBAGE"));
+
+
+     let res = parse_sql_statements("CREATE TABLE t (a int NOT NULL CONSTRAINT foo)");
+     assert!(res
+        .unwrap_err()
+        .to_string()
+        .contains("Expected constraint details after CONSTRAINT <name>"));
 }
 
 #[test]