Don't fail parsing a column definition with unexpected tokens

Since PR https://github.com/ballista-compute/sqlparser-rs/pull/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 https://github.com/ballista-compute/sqlparser-rs/pull/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.
This commit is contained in:
Nickolay Ponomarev 2020-08-10 17:12:33 +03:00
parent 23f5c7e7ce
commit 66505ebf9e
2 changed files with 40 additions and 27 deletions

View file

@ -1228,10 +1228,21 @@ impl<'a> Parser<'a> {
};
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,
@ -1241,23 +1252,17 @@ impl<'a> Parser<'a> {
})
}
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
@ -1276,32 +1281,34 @@ impl<'a> Parser<'a> {
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)
&& dialect_of!(self is MySqlDialect | GenericDialect)
{
// 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)
&& dialect_of!(self is SQLiteDialect | GenericDialect)
{
// 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> {

View file

@ -1142,7 +1142,13 @@ 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]