From c95b2401b2e97feec439f3e2d38997e14039cca3 Mon Sep 17 00:00:00 2001
From: WenyXu <wenymedia@gmail.com>
Date: Wed, 20 Nov 2024 11:24:28 +0000
Subject: [PATCH] test: add tests

---
 src/sql/src/parsers/alter_parser.rs           | 42 ++++++++++++++++---
 src/sql/src/statements/alter.rs               |  2 +-
 .../common/alter/alter_table_options.result   |  2 +-
 .../common/alter/alter_table_options.sql      |  2 +-
 4 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/src/sql/src/parsers/alter_parser.rs b/src/sql/src/parsers/alter_parser.rs
index 58173faa2b1e..4fa2a3f865bf 100644
--- a/src/sql/src/parsers/alter_parser.rs
+++ b/src/sql/src/parsers/alter_parser.rs
@@ -557,7 +557,7 @@ mod tests {
         }
     }
 
-    fn check_parse_alter_table(sql: &str, expected: &[(&str, &str)]) {
+    fn check_parse_alter_table_set_options(sql: &str, expected: &[(&str, &str)]) {
         let result =
             ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
                 .unwrap();
@@ -578,18 +578,36 @@ mod tests {
         assert_eq!(expected, &res);
     }
 
+    fn check_parse_alter_table_unset_options(sql: &str, expected: &[&str]) {
+        let result =
+            ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
+                .unwrap();
+        assert_eq!(1, result.len());
+        let Statement::Alter(alter) = &result[0] else {
+            unreachable!()
+        };
+        assert_eq!("test_table", alter.table_name.0[0].value);
+        let AlterTableOperation::UnsetTableOptions { keys } = &alter.alter_operation else {
+            unreachable!()
+        };
+
+        assert_eq!(sql, alter.to_string());
+        let res = keys.iter().map(|o| o.to_string()).collect::<Vec<_>>();
+        assert_eq!(expected, &res);
+    }
+
     #[test]
-    fn test_parse_alter_column() {
-        check_parse_alter_table("ALTER TABLE test_table SET 'a'='A'", &[("a", "A")]);
-        check_parse_alter_table(
+    fn test_parse_alter_table_set_options() {
+        check_parse_alter_table_set_options("ALTER TABLE test_table SET 'a'='A'", &[("a", "A")]);
+        check_parse_alter_table_set_options(
             "ALTER TABLE test_table SET 'a'='A','b'='B'",
             &[("a", "A"), ("b", "B")],
         );
-        check_parse_alter_table(
+        check_parse_alter_table_set_options(
             "ALTER TABLE test_table SET 'a'='A','b'='B','c'='C'",
             &[("a", "A"), ("b", "B"), ("c", "C")],
         );
-        check_parse_alter_table("ALTER TABLE test_table SET 'a'=NULL", &[("a", "")]);
+        check_parse_alter_table_set_options("ALTER TABLE test_table SET 'a'=NULL", &[("a", "")]);
 
         ParserContext::create_with_dialect(
             "ALTER TABLE test_table SET a INTEGER",
@@ -599,6 +617,18 @@ mod tests {
         .unwrap_err();
     }
 
+    #[test]
+    fn test_parse_alter_table_unset_options() {
+        check_parse_alter_table_unset_options("ALTER TABLE test_table UNSET 'a'", &["a"]);
+        check_parse_alter_table_unset_options("ALTER TABLE test_table UNSET 'a','b'", &["a", "b"]);
+        ParserContext::create_with_dialect(
+            "ALTER TABLE test_table UNSET a INTEGER",
+            &GreptimeDbDialect {},
+            ParseOptions::default(),
+        )
+        .unwrap_err();
+    }
+
     #[test]
     fn test_parse_alter_column_fulltext() {
         let sql = "ALTER TABLE test_table MODIFY COLUMN a SET FULLTEXT WITH(analyzer='English',case_sensitive='false')";
diff --git a/src/sql/src/statements/alter.rs b/src/sql/src/statements/alter.rs
index 9b79ff55b371..543c6e827294 100644
--- a/src/sql/src/statements/alter.rs
+++ b/src/sql/src/statements/alter.rs
@@ -135,7 +135,7 @@ impl Display for AlterTableOperation {
                 write!(f, "SET {kvs}")
             }
             AlterTableOperation::UnsetTableOptions { keys } => {
-                let keys = keys.join(",");
+                let keys = keys.iter().map(|k| format!("'{k}'")).join(",");
                 write!(f, "UNSET {keys}")
             }
             AlterTableOperation::SetColumnFulltext {
diff --git a/tests/cases/standalone/common/alter/alter_table_options.result b/tests/cases/standalone/common/alter/alter_table_options.result
index 53e765b0d23a..0cd05c96212e 100644
--- a/tests/cases/standalone/common/alter/alter_table_options.result
+++ b/tests/cases/standalone/common/alter/alter_table_options.result
@@ -186,7 +186,7 @@ SHOW CREATE TABLE ato;
 |       | )                                                  |
 +-------+----------------------------------------------------+
 
-ALTER TABLE ato SET 'compaction.twcs.time_window'='';
+ALTER TABLE ato UNSET 'compaction.twcs.time_window';
 
 Affected Rows: 0
 
diff --git a/tests/cases/standalone/common/alter/alter_table_options.sql b/tests/cases/standalone/common/alter/alter_table_options.sql
index d2ad4546c97d..a6cdb1de6f98 100644
--- a/tests/cases/standalone/common/alter/alter_table_options.sql
+++ b/tests/cases/standalone/common/alter/alter_table_options.sql
@@ -42,7 +42,7 @@ ALTER TABLE ato SET 'compaction.twcs.max_inactive_window_runs'='6';
 
 SHOW CREATE TABLE ato;
 
-ALTER TABLE ato SET 'compaction.twcs.time_window'='';
+ALTER TABLE ato UNSET 'compaction.twcs.time_window';
 
 SHOW CREATE TABLE ato;