rust/greptimedb
1
0
Fork 0
mirror of https://github.com/GreptimeTeam/greptimedb.git synced 2026-01-08 14:22:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: bugs introduced by alter table options (#4953)

Browse Source 
* fix: ChangeTableOptions display

* fix: partition number disappear after altering table options

* Update src/table/src/metadata.rs

Co-authored-by: Ruihang Xia <waynestxia@gmail.com>

---------

Co-authored-by: Lei, HUANG <6406592+v0y4g3r@users.noreply.github.com>
Co-authored-by: Ruihang Xia <waynestxia@gmail.com>
This commit is contained in:
dennis zhuang
2024-11-07 03:56:49 +08:00
committed by GitHub
parent 22a662f6bc
commit 305767e226
5 changed files with 108 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/sql/src/parsers/alter_parser.rs
View File

@@ -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",

19
src/sql/src/statements/alter.rs
View File

@@ -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 <table attrs key> = <table attr value>`
/// `SET <table attrs key> = <table attr value>`
ChangeTableOptions { options: Vec<ChangeTableOption> },
/// `DROP COLUMN <name>`
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(())
}
}

25
src/table/src/metadata.rs
View File

@@ -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)

69
tests/cases/standalone/common/alter/alter_table_options.result
View File

@@ -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

22
tests/cases/standalone/common/alter/alter_table_options.sql
View File

@@ -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;
SHOW CREATE TABLE ato;
ALTER TABLE ato SET 'ttl'='0d';
SELECT i FROM ato;
SHOW CREATE TABLE ato;
DROP TABLE ato;
ALTER TABLE ato SET 'ttl'='1s';
SHOW CREATE TABLE ato;
SELECT i FROM ato;
DROP TABLE ato;