refactor: refactor alter parser (#4933)

refactor: alter parser
2026-05-27 10:20:38 +00:00 · 2024-11-04 17:00:30 +08:00
parent 4ab6dc2825
commit fb822987a9
1 changed files with 117 additions and 74 deletions
--- a/src/sql/src/parsers/alter_parser.rs
+++ b/src/sql/src/parsers/alter_parser.rs
@@ -25,91 +25,131 @@ use crate::statements::statement::Statement;

 impl ParserContext<'_> {
    pub(crate) fn parse_alter(&mut self) -> Result<Statement> {
-        let alter_table = self.parse_alter_table().context(error::SyntaxSnafu)?;
+        let alter_table = self.parse_alter_table()?;
        Ok(Statement::Alter(alter_table))
    }

-    fn parse_alter_table(&mut self) -> std::result::Result<AlterTable, ParserError> {
+    fn parse_alter_table(&mut self) -> Result<AlterTable> {
        self.parser
-            .expect_keywords(&[Keyword::ALTER, Keyword::TABLE])?;
+            .expect_keywords(&[Keyword::ALTER, Keyword::TABLE])
+            .context(error::SyntaxSnafu)?;

-        let raw_table_name = self.parser.parse_object_name(false)?;
+        let raw_table_name = self
+            .parser
+            .parse_object_name(false)
+            .context(error::SyntaxSnafu)?;
        let table_name = Self::canonicalize_object_name(raw_table_name);

-        let alter_operation = if self.parser.parse_keyword(Keyword::ADD) {
-            if let Some(constraint) = self.parser.parse_optional_table_constraint()? {
-                AlterTableOperation::AddConstraint(constraint)
-            } else {
-                let _ = self.parser.parse_keyword(Keyword::COLUMN);
-                let mut column_def = self.parser.parse_column_def()?;
-                column_def.name = Self::canonicalize_identifier(column_def.name);
-                let location = if self.parser.parse_keyword(Keyword::FIRST) {
-                    Some(AddColumnLocation::First)
-                } else if let Token::Word(word) = self.parser.peek_token().token {
-                    if word.value.eq_ignore_ascii_case("AFTER") {
-                        let _ = self.parser.next_token();
-                        let name = Self::canonicalize_identifier(self.parse_identifier()?);
-                        Some(AddColumnLocation::After {
-                            column_name: name.value,
-                        })
-                    } else {
-                        None
-                    }
+        let alter_operation = match self.parser.peek_token().token {
+            Token::Word(w) => {
+                if w.value.eq_ignore_ascii_case("MODIFY") {
+                    self.parse_alter_table_modify()?
                } else {
-                    None
-                };
-                AlterTableOperation::AddColumn {
-                    column_def,
-                    location,
+                    match w.keyword {
+                        Keyword::ADD => self.parse_alter_table_add()?,
+                        Keyword::DROP => {
+                            let _ = self.parser.next_token();
+                            self.parser
+                                .expect_keyword(Keyword::COLUMN)
+                                .context(error::SyntaxSnafu)?;
+                            let name = Self::canonicalize_identifier(
+                                self.parse_identifier().context(error::SyntaxSnafu)?,
+                            );
+                            AlterTableOperation::DropColumn { name }
+                        }
+                        Keyword::RENAME => {
+                            let _ = self.parser.next_token();
+                            let new_table_name_obj_raw =
+                                self.parse_object_name().context(error::SyntaxSnafu)?;
+                            let new_table_name_obj =
+                                Self::canonicalize_object_name(new_table_name_obj_raw);
+                            let new_table_name = match &new_table_name_obj.0[..] {
+                                [table] => table.value.clone(),
+                                _ => {
+                                    return Err(ParserError::ParserError(format!(
+                                        "expect table name, actual: {new_table_name_obj}"
+                                    )))
+                                    .context(error::SyntaxSnafu)
+                                }
+                            };
+                            AlterTableOperation::RenameTable { new_table_name }
+                        }
+                        Keyword::SET => {
+                            let _ = self.parser.next_token();
+                            let options = self
+                                .parser
+                                .parse_comma_separated(parse_string_options)
+                                .context(error::SyntaxSnafu)?
+                                .into_iter()
+                                .map(|(key, value)| ChangeTableOption { key, value })
+                                .collect();
+                            AlterTableOperation::ChangeTableOptions { options }
+                        }
+                        _ => self.expected(
+                            "ADD or DROP or MODIFY or RENAME or SET after ALTER TABLE",
+                            self.parser.peek_token(),
+                        )?,
+                    }
                }
            }
-        } else if self.parser.parse_keyword(Keyword::DROP) {
-            if self.parser.parse_keyword(Keyword::COLUMN) {
-                let name = Self::canonicalize_identifier(self.parse_identifier()?);
-                AlterTableOperation::DropColumn { name }
-            } else {
-                return Err(ParserError::ParserError(format!(
-                    "expect keyword COLUMN after ALTER TABLE DROP, found {}",
-                    self.parser.peek_token()
-                )));
-            }
-        } else if self.consume_token("MODIFY") {
-            let _ = self.parser.parse_keyword(Keyword::COLUMN);
-            let column_name = Self::canonicalize_identifier(self.parser.parse_identifier(false)?);
-            let target_type = self.parser.parse_data_type()?;
-
-            AlterTableOperation::ChangeColumnType {
-                column_name,
-                target_type,
-            }
-        } else if self.parser.parse_keyword(Keyword::RENAME) {
-            let new_table_name_obj_raw = self.parse_object_name()?;
-            let new_table_name_obj = Self::canonicalize_object_name(new_table_name_obj_raw);
-            let new_table_name = match &new_table_name_obj.0[..] {
-                [table] => table.value.clone(),
-                _ => {
-                    return Err(ParserError::ParserError(format!(
-                        "expect table name, actual: {new_table_name_obj}"
-                    )))
-                }
-            };
-            AlterTableOperation::RenameTable { new_table_name }
-        } else if self.parser.parse_keyword(Keyword::SET) {
-            let options = self
-                .parser
-                .parse_comma_separated(parse_string_options)?
-                .into_iter()
-                .map(|(key, value)| ChangeTableOption { key, value })
-                .collect();
-            AlterTableOperation::ChangeTableOptions { options }
-        } else {
-            return Err(ParserError::ParserError(format!(
-                "expect keyword ADD or DROP or MODIFY or RENAME after ALTER TABLE, found {}",
-                self.parser.peek_token()
-            )));
+            unexpected => self.unsupported(unexpected.to_string())?,
        };
        Ok(AlterTable::new(table_name, alter_operation))
    }
+
+    fn parse_alter_table_add(&mut self) -> Result<AlterTableOperation> {
+        let _ = self.parser.next_token();
+        if let Some(constraint) = self
+            .parser
+            .parse_optional_table_constraint()
+            .context(error::SyntaxSnafu)?
+        {
+            Ok(AlterTableOperation::AddConstraint(constraint))
+        } else {
+            let _ = self.parser.parse_keyword(Keyword::COLUMN);
+            let mut column_def = self.parser.parse_column_def().context(error::SyntaxSnafu)?;
+            column_def.name = Self::canonicalize_identifier(column_def.name);
+            let location = if self.parser.parse_keyword(Keyword::FIRST) {
+                Some(AddColumnLocation::First)
+            } else if let Token::Word(word) = self.parser.peek_token().token {
+                if word.value.eq_ignore_ascii_case("AFTER") {
+                    let _ = self.parser.next_token();
+                    let name = Self::canonicalize_identifier(
+                        self.parse_identifier().context(error::SyntaxSnafu)?,
+                    );
+                    Some(AddColumnLocation::After {
+                        column_name: name.value,
+                    })
+                } else {
+                    None
+                }
+            } else {
+                None
+            };
+            Ok(AlterTableOperation::AddColumn {
+                column_def,
+                location,
+            })
+        }
+    }
+
+    fn parse_alter_table_modify(&mut self) -> Result<AlterTableOperation> {
+        let _ = self.parser.next_token();
+        self.parser
+            .expect_keyword(Keyword::COLUMN)
+            .context(error::SyntaxSnafu)?;
+        let column_name = Self::canonicalize_identifier(
+            self.parser
+                .parse_identifier(false)
+                .context(error::SyntaxSnafu)?,
+        );
+        let target_type = self.parser.parse_data_type().context(error::SyntaxSnafu)?;
+
+        Ok(AlterTableOperation::ChangeColumnType {
+            column_name,
+            target_type,
+        })
+    }
 }

 fn parse_string_options(parser: &mut Parser) -> std::result::Result<(String, String), ParserError> {
@@ -259,7 +299,10 @@ mod tests {
            ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
                .unwrap_err();
        let err = result.output_msg();
-        assert!(err.contains("expect keyword COLUMN after ALTER TABLE DROP"));
+        assert_eq!(
+            err,
+            "sql parser error: Expected COLUMN, found: a at Line: 1, Column 30"
+        );

        let sql = "ALTER TABLE my_metric_1 DROP COLUMN a";
        let mut result =
@@ -404,7 +447,7 @@ mod tests {
            ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
                .unwrap_err();
        let err = result.output_msg();
-        assert!(err.contains("expect keyword ADD or DROP or MODIFY or RENAME after ALTER TABLE"));
+        assert_eq!(err, "sql parser error: Expected ADD or DROP or MODIFY or RENAME or SET after ALTER TABLE, found: table_t");

        let sql = "ALTER TABLE test_table RENAME table_t";
        let mut result =