diff --git a/src/sql/src/parsers/alter_parser.rs b/src/sql/src/parsers/alter_parser.rs index 9df3a50accaf..bcb63881e246 100644 --- a/src/sql/src/parsers/alter_parser.rs +++ b/src/sql/src/parsers/alter_parser.rs @@ -486,6 +486,8 @@ mod tests { let AlterTableOperation::ChangeTableOptions { options } = &alter.alter_operation else { unreachable!() }; + + assert_eq!(sql, alter.to_string()); let res = options .iter() .map(|o| (o.key.as_str(), o.value.as_str())) @@ -495,16 +497,16 @@ mod tests { #[test] fn test_parse_alter_column() { - check_parse_alter_table("ALTER TABLE test_table SET 'a'='A';", &[("a", "A")]); + check_parse_alter_table("ALTER TABLE test_table SET 'a'='A'", &[("a", "A")]); check_parse_alter_table( "ALTER TABLE test_table SET 'a'='A','b'='B'", &[("a", "A"), ("b", "B")], ); check_parse_alter_table( - "ALTER TABLE test_table SET 'a'='A','b'='B','c'='C';", + "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("ALTER TABLE test_table SET 'a'=NULL", &[("a", "")]); ParserContext::create_with_dialect( "ALTER TABLE test_table SET a INTEGER", diff --git a/src/sql/src/statements/alter.rs b/src/sql/src/statements/alter.rs index 3270d002c8a6..a32f7b55cdf2 100644 --- a/src/sql/src/statements/alter.rs +++ b/src/sql/src/statements/alter.rs @@ -16,6 +16,7 @@ use std::fmt::{Debug, Display}; use api::v1; use common_query::AddColumnLocation; +use itertools::Itertools; use sqlparser::ast::{ColumnDef, DataType, Ident, ObjectName, TableConstraint}; use sqlparser_derive::{Visit, VisitMut}; @@ -68,7 +69,7 @@ pub enum AlterTableOperation { column_name: Ident, target_type: DataType, }, - /// `MODIFY =
` + /// `SET
=
` ChangeTableOptions { options: Vec }, /// `DROP COLUMN ` DropColumn { name: Ident }, @@ -101,9 +102,19 @@ impl Display for AlterTableOperation { write!(f, r#"MODIFY COLUMN {column_name} {target_type}"#) } AlterTableOperation::ChangeTableOptions { options } => { - for ChangeTableOption { key, value } in options { - write!(f, r#"MODIFY '{key}'='{value}', "#)?; - } + let kvs = options + .iter() + .map(|ChangeTableOption { key, value }| { + if !value.is_empty() { + format!("'{key}'='{value}'") + } else { + format!("'{key}'=NULL") + } + }) + .join(","); + + write!(f, "SET {kvs}")?; + Ok(()) } } diff --git a/src/table/src/metadata.rs b/src/table/src/metadata.rs index 3e7521750866..2b56e214cb77 100644 --- a/src/table/src/metadata.rs +++ b/src/table/src/metadata.rs @@ -201,15 +201,7 @@ impl TableMeta { self.change_column_types(table_name, columns) } // No need to rebuild table meta when renaming tables. - AlterKind::RenameTable { .. } => { - let mut meta_builder = TableMetaBuilder::default(); - let _ = meta_builder - .schema(self.schema.clone()) - .primary_key_indices(self.primary_key_indices.clone()) - .engine(self.engine.clone()) - .next_column_id(self.next_column_id); - Ok(meta_builder) - } + AlterKind::RenameTable { .. } => Ok(self.new_meta_builder()), AlterKind::ChangeTableOptions { options } => self.change_table_options(options), } } @@ -229,13 +221,9 @@ impl TableMeta { } } } - let mut builder = TableMetaBuilder::default(); - builder - .options(new_options) - .schema(self.schema.clone()) - .primary_key_indices(self.primary_key_indices.clone()) - .engine(self.engine.clone()) - .next_column_id(self.next_column_id); + let mut builder = self.new_meta_builder(); + builder.options(new_options); + Ok(builder) } @@ -266,9 +254,14 @@ impl TableMeta { Ok(desc) } + /// Create a [`TableMetaBuilder`]. + /// + /// Note: please always use this function to create the builder. fn new_meta_builder(&self) -> TableMetaBuilder { let mut builder = TableMetaBuilder::default(); let _ = builder + .schema(self.schema.clone()) + .primary_key_indices(self.primary_key_indices.clone()) .engine(&self.engine) .options(self.options.clone()) .created_on(self.created_on) diff --git a/tests/cases/standalone/common/alter/alter_table_options.result b/tests/cases/standalone/common/alter/alter_table_options.result index 13daf0f89b23..8e31766f0108 100644 --- a/tests/cases/standalone/common/alter/alter_table_options.result +++ b/tests/cases/standalone/common/alter/alter_table_options.result @@ -1,11 +1,33 @@ -CREATE TABLE ato(i INTEGER, j TIMESTAMP TIME INDEX); +CREATE TABLE ato(i INTEGER, j TIMESTAMP TIME INDEX, PRIMARY KEY(i)); Affected Rows: 0 +INSERT INTO ato VALUES(1, now()), (2, now()); + +Affected Rows: 2 + +SELECT i FROM ato; + ++---+ +| i | ++---+ +| 1 | +| 2 | ++---+ + ALTER TABLE ato SET 'ttl'='1d'; Affected Rows: 0 +SELECT i FROM ato; + ++---+ +| i | ++---+ +| 1 | +| 2 | ++---+ + SHOW CREATE TABLE ato; +-------+------------------------------------+ @@ -14,7 +36,8 @@ SHOW CREATE TABLE ato; | ato | CREATE TABLE IF NOT EXISTS "ato" ( | | | "i" INT NULL, | | | "j" TIMESTAMP(3) NOT NULL, | -| | TIME INDEX ("j") | +| | TIME INDEX ("j"), | +| | PRIMARY KEY ("i") | | | ) | | | | | | ENGINE=mito | @@ -27,6 +50,15 @@ ALTER TABLE ato SET 'ttl'='2d'; Affected Rows: 0 +SELECT i FROM ato; + ++---+ +| i | ++---+ +| 1 | +| 2 | ++---+ + SHOW CREATE TABLE ato; +-------+------------------------------------+ @@ -35,7 +67,8 @@ SHOW CREATE TABLE ato; | ato | CREATE TABLE IF NOT EXISTS "ato" ( | | | "i" INT NULL, | | | "j" TIMESTAMP(3) NOT NULL, | -| | TIME INDEX ("j") | +| | TIME INDEX ("j"), | +| | PRIMARY KEY ("i") | | | ) | | | | | | ENGINE=mito | @@ -48,6 +81,15 @@ ALTER TABLE ato SET 'ttl'=NULL; Affected Rows: 0 +SELECT i FROM ato; + ++---+ +| i | ++---+ +| 1 | +| 2 | ++---+ + SHOW CREATE TABLE ato; +-------+------------------------------------+ @@ -56,14 +98,15 @@ SHOW CREATE TABLE ato; | ato | CREATE TABLE IF NOT EXISTS "ato" ( | | | "i" INT NULL, | | | "j" TIMESTAMP(3) NOT NULL, | -| | TIME INDEX ("j") | +| | TIME INDEX ("j"), | +| | PRIMARY KEY ("i") | | | ) | | | | | | ENGINE=mito | | | | +-------+------------------------------------+ -ALTER TABLE ato SET 'ttl'='0d'; +ALTER TABLE ato SET 'ttl'='1s'; Affected Rows: 0 @@ -75,13 +118,25 @@ SHOW CREATE TABLE ato; | ato | CREATE TABLE IF NOT EXISTS "ato" ( | | | "i" INT NULL, | | | "j" TIMESTAMP(3) NOT NULL, | -| | TIME INDEX ("j") | +| | TIME INDEX ("j"), | +| | PRIMARY KEY ("i") | | | ) | | | | | | ENGINE=mito | -| | | +| | WITH( | +| | ttl = '1s' | +| | ) | +-------+------------------------------------+ +SELECT i FROM ato; + ++---+ +| i | ++---+ +| 1 | +| 2 | ++---+ + DROP TABLE ato; 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 4b30a8c28f1d..aab558543de2 100644 --- a/tests/cases/standalone/common/alter/alter_table_options.sql +++ b/tests/cases/standalone/common/alter/alter_table_options.sql @@ -1,19 +1,31 @@ -CREATE TABLE ato(i INTEGER, j TIMESTAMP TIME INDEX); +CREATE TABLE ato(i INTEGER, j TIMESTAMP TIME INDEX, PRIMARY KEY(i)); + +INSERT INTO ato VALUES(1, now()), (2, now()); + +SELECT i FROM ato; ALTER TABLE ato SET 'ttl'='1d'; +SELECT i FROM ato; + SHOW CREATE TABLE ato; ALTER TABLE ato SET 'ttl'='2d'; +SELECT i FROM ato; + SHOW CREATE TABLE ato; ALTER TABLE ato SET 'ttl'=NULL; +SELECT i FROM ato; + SHOW CREATE TABLE ato; -ALTER TABLE ato SET 'ttl'='0d'; +ALTER TABLE ato SET 'ttl'='1s'; SHOW CREATE TABLE ato; -DROP TABLE ato; \ No newline at end of file +SELECT i FROM ato; + +DROP TABLE ato;