From 2705c214c2e41fc2d85f1eec235a413c01cf96d4 Mon Sep 17 00:00:00 2001 From: mashuai Date: Tue, 16 Jun 2020 19:55:13 +0800 Subject: [PATCH 1/6] implement alter table for sqlite grammer: https://www.sqlite.org/lang_altertable.html --- src/ast/ddl.rs | 47 ++++++++++++++++++++++++++++++++- src/dialect/keywords.rs | 1 + src/parser.rs | 50 +++++++++++++++++++++++++++++++++-- tests/sqlparser_common.rs | 55 ++++++++++++++++++++++++++++++++++++--- 4 files changed, 147 insertions(+), 6 deletions(-) diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs index d7503ba77..492cb78af 100644 --- a/src/ast/ddl.rs +++ b/src/ast/ddl.rs @@ -23,15 +23,60 @@ use std::fmt; pub enum AlterTableOperation { /// `ADD ` AddConstraint(TableConstraint), + AddColumn { + has_column_identifier: bool, + column_def: ColumnDef, + }, /// TODO: implement `DROP CONSTRAINT ` - DropConstraint { name: Ident }, + DropConstraint { + name: Ident, + }, + RenameColumn { + has_column_identifier: bool, + old_column_name: Ident, + new_column_name: Ident, + }, + RenameTable { + table_name: Ident, + }, } impl fmt::Display for AlterTableOperation { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { AlterTableOperation::AddConstraint(c) => write!(f, "ADD {}", c), + AlterTableOperation::AddColumn { + has_column_identifier, + column_def, + } => write!( + f, + "ADD {}{}", + if *has_column_identifier { + "COLUMN " + } else { + "" + }, + column_def.to_string() + ), AlterTableOperation::DropConstraint { name } => write!(f, "DROP CONSTRAINT {}", name), + AlterTableOperation::RenameColumn { + has_column_identifier, + old_column_name, + new_column_name, + } => write!( + f, + "RENAME {}{} TO {}", + if *has_column_identifier { + "COLUMN " + } else { + "" + }, + old_column_name, + new_column_name + ), + AlterTableOperation::RenameTable { table_name } => { + write!(f, "RENAME TO {}", table_name) + } } } } diff --git a/src/dialect/keywords.rs b/src/dialect/keywords.rs index f5e75f74c..1d84fa5f5 100644 --- a/src/dialect/keywords.rs +++ b/src/dialect/keywords.rs @@ -340,6 +340,7 @@ define_keywords!( REGR_SXY, REGR_SYY, RELEASE, + RENAME, REPEATABLE, RESTRICT, RESULT, diff --git a/src/parser.rs b/src/parser.rs index 1763afb49..97b72bee6 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1122,6 +1122,29 @@ impl Parser { }) } + fn parse_column(&mut self) -> Result { + let name = self.parse_identifier()?; + let data_type = self.parse_data_type()?; + let collation = if self.parse_keyword(Keyword::COLLATE) { + Some(self.parse_object_name()?) + } else { + None + }; + let mut options = vec![]; + loop { + match self.peek_token() { + Token::EOF => break, + _ => options.push(self.parse_column_option_def()?), + } + } + Ok(ColumnDef { + name, + data_type, + collation, + options, + }) + } + fn parse_columns(&mut self) -> Result<(Vec, Vec), ParserError> { let mut columns = vec![]; let mut constraints = vec![]; @@ -1318,10 +1341,33 @@ impl Parser { if let Some(constraint) = self.parse_optional_table_constraint()? { AlterTableOperation::AddConstraint(constraint) } else { - return self.expected("a constraint in ALTER TABLE .. ADD", self.peek_token()); + let has_column_identifier = self.parse_keyword(Keyword::COLUMN); + let column_def = self.parse_column()?; + AlterTableOperation::AddColumn { + has_column_identifier, + column_def, + } + } + } else if self.parse_keyword(Keyword::RENAME) { + if self.parse_keyword(Keyword::TO) { + let table_name = self.parse_identifier()?; + AlterTableOperation::RenameTable { table_name } + } else { + let has_column_identifier = self.parse_keyword(Keyword::COLUMN); + let old_column_name = self.parse_identifier()?; + self.expect_keyword(Keyword::TO)?; + let new_column_name = self.parse_identifier()?; + AlterTableOperation::RenameColumn { + has_column_identifier, + old_column_name, + new_column_name, + } } } else { - return self.expected("ADD after ALTER TABLE", self.peek_token()); + return self.expected( + "ADD,RENAME TO or RENAME after ALTER TABLE", + self.peek_token(), + ); }; Ok(Statement::AlterTable { name: table_name, diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index bff131334..eb4da4a83 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -1313,6 +1313,57 @@ fn parse_create_table_empty() { let _ = verified_stmt("CREATE TABLE t ()"); } +#[test] +fn parse_alter_table() { + let add_column = "ALTER TABLE tab ADD COLUMN foo TEXT"; + match verified_stmt(add_column) { + Statement::AlterTable { + name, + operation: + AlterTableOperation::AddColumn { + has_column_identifier, + column_def, + }, + } => { + assert_eq!("tab", name.to_string()); + assert_eq!(has_column_identifier, true); + assert_eq!("foo", column_def.name.to_string()); + } + _ => unreachable!(), + }; + + let rename_table = "ALTER TABLE tab RENAME TO new_tab"; + match verified_stmt(rename_table) { + Statement::AlterTable { + name, + operation: AlterTableOperation::RenameTable { table_name }, + } => { + assert_eq!("tab", name.to_string()); + assert_eq!("new_tab", table_name.to_string()) + } + _ => unreachable!(), + }; + + let rename_column = "ALTER TABLE tab RENAME COLUMN foo TO new_foo"; + match verified_stmt(rename_column) { + Statement::AlterTable { + name, + operation: + AlterTableOperation::RenameColumn { + has_column_identifier, + old_column_name, + new_column_name, + }, + } => { + assert_eq!("tab", name.to_string()); + assert_eq!(has_column_identifier, true); + assert_eq!(old_column_name.to_string(), "foo"); + assert_eq!(new_column_name.to_string(), "new_foo"); + } + _ => unreachable!(), + } +} + #[test] fn parse_alter_table_constraints() { check_one("CONSTRAINT address_pkey PRIMARY KEY (address_id)"); @@ -1347,9 +1398,7 @@ fn parse_alter_table_constraints() { fn parse_bad_constraint() { let res = parse_sql_statements("ALTER TABLE tab ADD"); assert_eq!( - ParserError::ParserError( - "Expected a constraint in ALTER TABLE .. ADD, found: EOF".to_string() - ), + ParserError::ParserError("Expected identifier, found: EOF".to_string()), res.unwrap_err() ); From 84e2f7d1a0322e624f55fd3a2184af7a374c475a Mon Sep 17 00:00:00 2001 From: mashuai Date: Tue, 16 Jun 2020 22:53:56 +0800 Subject: [PATCH 2/6] resolve comment --- src/ast/ddl.rs | 44 ++++++++++----------------------------- src/parser.rs | 15 ++++--------- tests/sqlparser_common.rs | 10 ++------- 3 files changed, 17 insertions(+), 52 deletions(-) diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs index 492cb78af..3b8325204 100644 --- a/src/ast/ddl.rs +++ b/src/ast/ddl.rs @@ -23,56 +23,34 @@ use std::fmt; pub enum AlterTableOperation { /// `ADD ` AddConstraint(TableConstraint), - AddColumn { - has_column_identifier: bool, - column_def: ColumnDef, - }, + /// `ADD [ COLUMN ] ` + AddColumn { column_def: ColumnDef }, /// TODO: implement `DROP CONSTRAINT ` - DropConstraint { - name: Ident, - }, + DropConstraint { name: Ident }, + /// `RENAME [ COLUMN ] TO ` RenameColumn { - has_column_identifier: bool, old_column_name: Ident, new_column_name: Ident, }, - RenameTable { - table_name: Ident, - }, + /// `RENAME TO ` + RenameTable { table_name: Ident }, } impl fmt::Display for AlterTableOperation { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { AlterTableOperation::AddConstraint(c) => write!(f, "ADD {}", c), - AlterTableOperation::AddColumn { - has_column_identifier, - column_def, - } => write!( - f, - "ADD {}{}", - if *has_column_identifier { - "COLUMN " - } else { - "" - }, - column_def.to_string() - ), + AlterTableOperation::AddColumn { column_def } => { + write!(f, "ADD COLUMN {}", column_def.to_string()) + } AlterTableOperation::DropConstraint { name } => write!(f, "DROP CONSTRAINT {}", name), AlterTableOperation::RenameColumn { - has_column_identifier, old_column_name, new_column_name, } => write!( f, - "RENAME {}{} TO {}", - if *has_column_identifier { - "COLUMN " - } else { - "" - }, - old_column_name, - new_column_name + "RENAME COLUMN {} TO {}", + old_column_name, new_column_name ), AlterTableOperation::RenameTable { table_name } => { write!(f, "RENAME TO {}", table_name) diff --git a/src/parser.rs b/src/parser.rs index 97b72bee6..172de3f32 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1341,33 +1341,26 @@ impl Parser { if let Some(constraint) = self.parse_optional_table_constraint()? { AlterTableOperation::AddConstraint(constraint) } else { - let has_column_identifier = self.parse_keyword(Keyword::COLUMN); + let _ = self.parse_keyword(Keyword::COLUMN); let column_def = self.parse_column()?; - AlterTableOperation::AddColumn { - has_column_identifier, - column_def, - } + AlterTableOperation::AddColumn { column_def } } } else if self.parse_keyword(Keyword::RENAME) { if self.parse_keyword(Keyword::TO) { let table_name = self.parse_identifier()?; AlterTableOperation::RenameTable { table_name } } else { - let has_column_identifier = self.parse_keyword(Keyword::COLUMN); + let _ = self.parse_keyword(Keyword::COLUMN); let old_column_name = self.parse_identifier()?; self.expect_keyword(Keyword::TO)?; let new_column_name = self.parse_identifier()?; AlterTableOperation::RenameColumn { - has_column_identifier, old_column_name, new_column_name, } } } else { - return self.expected( - "ADD,RENAME TO or RENAME after ALTER TABLE", - self.peek_token(), - ); + return self.expected("ADD or RENAME after ALTER TABLE", self.peek_token()); }; Ok(Statement::AlterTable { name: table_name, diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index eb4da4a83..15050c6fb 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -1319,15 +1319,11 @@ fn parse_alter_table() { match verified_stmt(add_column) { Statement::AlterTable { name, - operation: - AlterTableOperation::AddColumn { - has_column_identifier, - column_def, - }, + operation: AlterTableOperation::AddColumn { column_def }, } => { assert_eq!("tab", name.to_string()); - assert_eq!(has_column_identifier, true); assert_eq!("foo", column_def.name.to_string()); + assert_eq!("TEXT", column_def.data_type.to_string()); } _ => unreachable!(), }; @@ -1350,13 +1346,11 @@ fn parse_alter_table() { name, operation: AlterTableOperation::RenameColumn { - has_column_identifier, old_column_name, new_column_name, }, } => { assert_eq!("tab", name.to_string()); - assert_eq!(has_column_identifier, true); assert_eq!(old_column_name.to_string(), "foo"); assert_eq!(new_column_name.to_string(), "new_foo"); } From 5e494871b0ea042efb9bbc39f7167d6e789d524f Mon Sep 17 00:00:00 2001 From: mashuai Date: Tue, 16 Jun 2020 23:26:21 +0800 Subject: [PATCH 3/6] reuse parse_column --- src/parser.rs | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 172de3f32..5ba04d84c 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1133,7 +1133,7 @@ impl Parser { let mut options = vec![]; loop { match self.peek_token() { - Token::EOF => break, + Token::EOF | Token::Comma | Token::RParen => break, _ => options.push(self.parse_column_option_def()?), } } @@ -1155,28 +1155,9 @@ impl Parser { loop { if let Some(constraint) = self.parse_optional_table_constraint()? { constraints.push(constraint); - } else if let Token::Word(column_name) = self.peek_token() { - self.next_token(); - let data_type = self.parse_data_type()?; - let collation = if self.parse_keyword(Keyword::COLLATE) { - Some(self.parse_object_name()?) - } else { - None - }; - let mut options = vec![]; - loop { - match self.peek_token() { - Token::EOF | Token::Comma | Token::RParen => break, - _ => options.push(self.parse_column_option_def()?), - } - } - - columns.push(ColumnDef { - name: column_name.to_ident(), - data_type, - collation, - options, - }); + } else if let Token::Word(_) = self.peek_token() { + let column_def = self.parse_column()?; + columns.push(column_def); } else { return self.expected("column name or constraint definition", self.peek_token()); } From a2d45d9d1f1f4f9fed71a1d53be167c535ea145b Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Tue, 16 Jun 2020 22:38:50 +0300 Subject: [PATCH 4/6] Rename parse_column -> parse_column_def to match the struct name --- src/parser.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 5ba04d84c..fb750afa0 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1122,7 +1122,7 @@ impl Parser { }) } - fn parse_column(&mut self) -> Result { + fn parse_column_def(&mut self) -> Result { let name = self.parse_identifier()?; let data_type = self.parse_data_type()?; let collation = if self.parse_keyword(Keyword::COLLATE) { @@ -1156,7 +1156,7 @@ impl Parser { if let Some(constraint) = self.parse_optional_table_constraint()? { constraints.push(constraint); } else if let Token::Word(_) = self.peek_token() { - let column_def = self.parse_column()?; + let column_def = self.parse_column_def()?; columns.push(column_def); } else { return self.expected("column name or constraint definition", self.peek_token()); @@ -1323,7 +1323,7 @@ impl Parser { AlterTableOperation::AddConstraint(constraint) } else { let _ = self.parse_keyword(Keyword::COLUMN); - let column_def = self.parse_column()?; + let column_def = self.parse_column_def()?; AlterTableOperation::AddColumn { column_def } } } else if self.parse_keyword(Keyword::RENAME) { From 1a840493e59a26f5931ee06f0c6a4cc4d8344083 Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Tue, 16 Jun 2020 22:39:23 +0300 Subject: [PATCH 5/6] Use the member names in doc comments --- src/ast/ddl.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs index 3b8325204..c1e66373b 100644 --- a/src/ast/ddl.rs +++ b/src/ast/ddl.rs @@ -23,16 +23,16 @@ use std::fmt; pub enum AlterTableOperation { /// `ADD ` AddConstraint(TableConstraint), - /// `ADD [ COLUMN ] ` + /// `ADD [ COLUMN ] ` AddColumn { column_def: ColumnDef }, /// TODO: implement `DROP CONSTRAINT ` DropConstraint { name: Ident }, - /// `RENAME [ COLUMN ] TO ` + /// `RENAME [ COLUMN ] TO ` RenameColumn { old_column_name: Ident, new_column_name: Ident, }, - /// `RENAME TO ` + /// `RENAME TO ` RenameTable { table_name: Ident }, } From 06d11ac0df52b92061a388c066b752d4e5c1a555 Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Tue, 16 Jun 2020 22:51:04 +0300 Subject: [PATCH 6/6] Update CHANGELOG --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 616f8774e..06b0220cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Check https://github.com/andygrove/sqlparser-rs/commits/master for undocumented - Use Token::EOF instead of Option (#195) - Make the units keyword following `INTERVAL '...'` optional (#184) - thanks @maxcountryman! - Generalize `DATE`/`TIME`/`TIMESTAMP` literals representation in the AST (`TypedString { data_type, value }`) and allow `DATE` and other keywords to be used as identifiers when not followed by a string (#187) - thanks @maxcountryman! +- Output DataType capitalized (`fmt::Display`) (#202) - thanks @Dandandan! ### Added - Support MSSQL `TOP () [ PERCENT ] [ WITH TIES ]` (#150) - thanks @alexkyllo! @@ -29,9 +30,11 @@ Check https://github.com/andygrove/sqlparser-rs/commits/master for undocumented - Support the string concatentation operator `||` (#178) - thanks @Dandandan! - Support bitwise AND (`&`), OR (`|`), XOR (`^`) (#181) - thanks @Dandandan! - Add serde support to AST structs and enums (#196) - thanks @panarch! +- Support `ALTER TABLE ADD COLUMN`, `RENAME COLUMN`, and `RENAME TO` (#203) - thanks @mashuai! ### Fixed - Report an error for unterminated string literals (#165) +- Make file format (`STORED AS`) case insensitive (#200) and don't allow quoting it (#201) - thanks @Dandandan! ## [0.5.0] - 2019-10-10