fix: ALTER COLUMN ... TYPE -> MODIFY COLUMN

GreptimeTeam · Apr 28, 2024 · 83efb1c · 83efb1c
1 parent b938f90
commit 83efb1c
Show file tree

Hide file tree

Showing 8 changed files with 59 additions and 59 deletions.
diff --git a/src/operator/src/expr_factory.rs b/src/operator/src/expr_factory.rs
@@ -662,7 +662,7 @@ mod tests {
 
     #[test]
     fn test_to_alter_change_column_type_expr() {
-        let sql = "ALTER TABLE monitor alter column mem_usage TYPE STRING;";
+        let sql = "ALTER TABLE monitor MODIFY mem_usage STRING;";
         let stmt =
             ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
                 .unwrap()

diff --git a/src/sql/src/parser.rs b/src/sql/src/parser.rs
@@ -187,8 +187,13 @@ impl<'a> ParserContext<'a> {
     }
 
     pub fn consume_token(&mut self, expected: &str) -> bool {
-        if self.peek_token_as_string().to_uppercase() == *expected.to_uppercase() {
-            let _ = self.parser.next_token();
+        Self::inner_consume_token(&mut self.parser, expected)
+    }
+
+    #[inline]
+    pub(crate) fn inner_consume_token(parser: &mut Parser<'a>, expected: &str) -> bool {
+        if Self::inner_peek_token_as_string(parser).to_uppercase() == *expected.to_uppercase() {
+            let _ = parser.next_token();
             true
         } else {
             false
@@ -197,7 +202,12 @@ impl<'a> ParserContext<'a> {
 
     #[inline]
     pub(crate) fn peek_token_as_string(&self) -> String {
-        self.parser.peek_token().to_string()
+        Self::inner_peek_token_as_string(&self.parser)
+    }
+
+    #[inline]
+    pub(crate) fn inner_peek_token_as_string(parser: &Parser<'a>) -> String {
+        parser.peek_token().to_string()
     }
 
     /// Canonicalize the identifier to lowercase if it's not quoted.

diff --git a/src/sql/src/parsers/alter_parser.rs b/src/sql/src/parsers/alter_parser.rs
@@ -73,25 +73,14 @@ impl<'a> ParserContext<'a> {
                     parser.peek_token()
                 )));
             }
-        } else if parser
-            .expect_keywords(&[Keyword::ALTER, Keyword::COLUMN])
-            .is_ok()
-        {
+        } else if ParserContext::inner_consume_token(parser, "MODIFY") {
+            let _ = parser.parse_keyword(Keyword::COLUMN);
             let column_name = Self::canonicalize_identifier(parser.parse_identifier(false)?);
+            let target_type = parser.parse_data_type()?;
 
-            if parser.parse_keyword(Keyword::TYPE) {
-                let target_type = parser.parse_data_type()?;
-
-                AlterTableOperation::ChangeColumnType {
-                    column_name,
-                    target_type,
-                }
-            } else {
-                return Err(ParserError::ParserError(format!(
-                    "expect keyword TYPE or TO after ALTER TABLE ALTER COLUMN {}, found {}",
-                    column_name,
-                    parser.peek_token()
-                )));
+            AlterTableOperation::ChangeColumnType {
+                column_name,
+                target_type,
             }
         } else if parser.parse_keyword(Keyword::RENAME) {
             let new_table_name_obj_raw = self.parse_object_name()?;
@@ -107,7 +96,7 @@ impl<'a> ParserContext<'a> {
             AlterTableOperation::RenameTable { new_table_name }
         } else {
             return Err(ParserError::ParserError(format!(
-                "expect keyword ADD or DROP or ALERT COLUMN or RENAME after ALTER TABLE, found {}",
+                "expect keyword ADD or DROP or MODIFY or RENAME after ALTER TABLE, found {}",
                 parser.peek_token()
             )));
         };
@@ -275,22 +264,25 @@ mod tests {
 
     #[test]
     fn test_parse_alter_change_column_type() {
-        let sql = "ALTER TABLE my_metric_1 ALTER COLUMN a STRING";
-        let result =
-            ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
-                .unwrap_err();
-
-        let err = result.output_msg();
-        assert!(err
-            .contains("expect keyword TYPE or TO after ALTER TABLE ALTER COLUMN a, found STRING"));
-
-        let sql = "ALTER TABLE my_metric_1 ALTER COLUMN a TYPE STRING";
-        let mut result =
-            ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
-                .unwrap();
-        assert_eq!(1, result.len());
-
-        let statement = result.remove(0);
+        let sql_1 = "ALTER TABLE my_metric_1 MODIFY COLUMN a STRING";
+        let result_1 = ParserContext::create_with_dialect(
+            sql_1,
+            &GreptimeDbDialect {},
+            ParseOptions::default(),
+        )
+        .unwrap();
+
+        let sql_2 = "ALTER TABLE my_metric_1 MODIFY a STRING";
+        let mut result_2 = ParserContext::create_with_dialect(
+            sql_2,
+            &GreptimeDbDialect {},
+            ParseOptions::default(),
+        )
+        .unwrap();
+        assert_eq!(result_1, result_2);
+        assert_eq!(1, result_2.len());
+
+        let statement = result_2.remove(0);
         assert_matches!(statement, Statement::Alter { .. });
         match statement {
             Statement::Alter(alter_table) => {
@@ -323,9 +315,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 ALERT COLUMN or RENAME after ALTER TABLE")
-        );
+        assert!(err.contains("expect keyword ADD or DROP or MODIFY or RENAME after ALTER TABLE"));
 
         let sql = "ALTER TABLE test_table RENAME table_t";
         let mut result =

diff --git a/src/sql/src/statements/alter.rs b/src/sql/src/statements/alter.rs
@@ -58,7 +58,7 @@ pub enum AlterTableOperation {
         column_def: ColumnDef,
         location: Option<AddColumnLocation>,
     },
-    /// `ALTER COLUMN <column_name> TYPE [target_type]`
+    /// `MODIFY <column_name> [target_type]`
     ChangeColumnType {
         column_name: Ident,
         target_type: DataType,
@@ -91,7 +91,7 @@ impl Display for AlterTableOperation {
                 column_name,
                 target_type,
             } => {
-                write!(f, r#"ALTER COLUMN {column_name} TYPE {target_type}"#)
+                write!(f, r#"MODIFY COLUMN {column_name} {target_type}"#)
             }
         }
     }
@@ -128,7 +128,7 @@ ALTER TABLE monitor ADD COLUMN app STRING DEFAULT 'shop' PRIMARY KEY"#,
             }
         }
 
-        let sql = r"alter table monitor alter column load_15 type string;";
+        let sql = r"alter table monitor modify column load_15 string;";
         let stmts =
             ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
                 .unwrap();
@@ -140,7 +140,7 @@ ALTER TABLE monitor ADD COLUMN app STRING DEFAULT 'shop' PRIMARY KEY"#,
                 let new_sql = format!("\n{}", set);
                 assert_eq!(
                     r#"
-ALTER TABLE monitor ALTER COLUMN load_15 TYPE STRING"#,
+ALTER TABLE monitor MODIFY COLUMN load_15 STRING"#,
                     &new_sql
                 );
             }

diff --git a/tests/cases/standalone/common/alter/change_col_type.result b/tests/cases/standalone/common/alter/change_col_type.result
@@ -6,23 +6,23 @@ INSERT INTO test VALUES (1, 1, 1, false), (2, 2, 2, true);
 
 Affected Rows: 2
 
-ALTER TABLE test ALTER COLUMN "I" TYPE STRING;
+ALTER TABLE test MODIFY "I" STRING;
 
 Error: 4002(TableColumnNotFound), Column I not exists in table test
 
-ALTER TABLE test ALTER COLUMN k TYPE DATE;
+ALTER TABLE test MODIFY k DATE;
 
 Error: 1004(InvalidArguments), Invalid alter table(test) request: column 'k' cannot be cast automatically to type 'Date'
 
-ALTER TABLE test ALTER COLUMN id TYPE STRING;
+ALTER TABLE test MODIFY id STRING;
 
 Error: 1004(InvalidArguments), Invalid alter table(test) request: Not allowed to change primary key index column 'id'
 
-ALTER TABLE test ALTER COLUMN j TYPE STRING;
+ALTER TABLE test MODIFY j STRING;
 
 Error: 1004(InvalidArguments), Invalid alter table(test) request: Not allowed to change timestamp index column 'j' datatype
 
-ALTER TABLE test ALTER COLUMN I TYPE STRING;
+ALTER TABLE test MODIFY I STRING;
 
 Affected Rows: 0
 
@@ -60,7 +60,7 @@ DESCRIBE test;
 | k      | Boolean              |     | YES  |         | FIELD         |
 +--------+----------------------+-----+------+---------+---------------+
 
-ALTER TABLE test ALTER COLUMN I TYPE INTEGER;
+ALTER TABLE test MODIFY I INTEGER;
 
 Affected Rows: 0
 

diff --git a/tests/cases/standalone/common/alter/change_col_type.sql b/tests/cases/standalone/common/alter/change_col_type.sql
@@ -2,15 +2,15 @@ CREATE TABLE test(id INTEGER PRIMARY KEY, i INTEGER NULL, j TIMESTAMP TIME INDEX
 
 INSERT INTO test VALUES (1, 1, 1, false), (2, 2, 2, true);
 
-ALTER TABLE test ALTER COLUMN "I" TYPE STRING;
+ALTER TABLE test MODIFY "I" STRING;
 
-ALTER TABLE test ALTER COLUMN k TYPE DATE;
+ALTER TABLE test MODIFY k DATE;
 
-ALTER TABLE test ALTER COLUMN id TYPE STRING;
+ALTER TABLE test MODIFY id STRING;
 
-ALTER TABLE test ALTER COLUMN j TYPE STRING;
+ALTER TABLE test MODIFY j STRING;
 
-ALTER TABLE test ALTER COLUMN I TYPE STRING;
+ALTER TABLE test MODIFY I STRING;
 
 SELECT * FROM test;
 
@@ -20,7 +20,7 @@ SELECT * FROM test;
 
 DESCRIBE test;
 
-ALTER TABLE test ALTER COLUMN I TYPE INTEGER;
+ALTER TABLE test MODIFY I INTEGER;
 
 SELECT * FROM test;
 

diff --git a/tests/cases/standalone/common/alter/change_col_type_not_null.result b/tests/cases/standalone/common/alter/change_col_type_not_null.result
@@ -15,7 +15,7 @@ SELECT * FROM test;
 | 1970-01-01T00:00:00.002 | 2 |
 +-------------------------+---+
 
-ALTER TABLE test ALTER COLUMN j TYPE STRING;
+ALTER TABLE test MODIFY j STRING;
 
 Error: 1004(InvalidArguments), Invalid alter table(test) request: column 'j' must be nullable to ensure safe conversion.
 

diff --git a/tests/cases/standalone/common/alter/change_col_type_not_null.sql b/tests/cases/standalone/common/alter/change_col_type_not_null.sql
@@ -4,7 +4,7 @@ INSERT INTO test VALUES (1, 1), (2, 2);
 
 SELECT * FROM test;
 
-ALTER TABLE test ALTER COLUMN j TYPE STRING;
+ALTER TABLE test MODIFY j STRING;
 
 SELECT * FROM test;