feat: alter database ttl (#5035)

* feat: alter databaset ttl

* fix: make clippy happy

* feat: add unset database option

* fix: happy ci

* fix: happy clippy

* chore: fmt toml

* fix: fix header

* refactor: introduce `AlterDatabaseKind`

* chore: apply suggestions from CR

* refactor: add unset database option support

* test: add unit tests

* test: add sqlness tests

* feat: invalidate schema name value cache

* Apply suggestions from code review

* chore: fmt

* chore: update error messages

* test: add more test cases

* test: add more test cases

* Apply suggestions from code review

* chore: apply suggestions from CR

---------

Co-authored-by: WenyXu <wenymedia@gmail.com>
This commit is contained in:
Yohan Wal
2024-11-21 20:41:41 +08:00
committed by GitHub
parent 3029b47a89
commit 5f8d849981
41 changed files with 1320 additions and 186 deletions

View File

@@ -24,6 +24,7 @@ datafusion-physical-expr.workspace = true
datafusion-sql.workspace = true
datatypes.workspace = true
hex = "0.4"
humantime.workspace = true
iso8601 = "0.6.1"
itertools.workspace = true
jsonb.workspace = true
@@ -33,6 +34,7 @@ serde_json.workspace = true
snafu.workspace = true
sqlparser.workspace = true
sqlparser_derive = "0.1"
store-api.workspace = true
table.workspace = true
[dev-dependencies]

View File

@@ -25,19 +25,78 @@ use sqlparser::tokenizer::Token;
use crate::error::{self, InvalidColumnOptionSnafu, Result, SetFulltextOptionSnafu};
use crate::parser::ParserContext;
use crate::parsers::utils::validate_column_fulltext_create_option;
use crate::statements::alter::{AlterTable, AlterTableOperation, TableOption};
use crate::statements::alter::{
AlterDatabase, AlterDatabaseOperation, AlterTable, AlterTableOperation, KeyValueOption,
};
use crate::statements::statement::Statement;
use crate::util::parse_option_string;
impl ParserContext<'_> {
pub(crate) fn parse_alter(&mut self) -> Result<Statement> {
let alter_table = self.parse_alter_table()?;
Ok(Statement::Alter(alter_table))
let _ = self.parser.expect_keyword(Keyword::ALTER);
match self.parser.peek_token().token {
Token::Word(w) => match w.keyword {
Keyword::DATABASE => self.parse_alter_database().map(Statement::AlterDatabase),
Keyword::TABLE => self.parse_alter_table().map(Statement::AlterTable),
_ => self.expected("DATABASE or TABLE after ALTER", self.parser.peek_token()),
},
unexpected => self.unsupported(unexpected.to_string()),
}
}
fn parse_alter_database(&mut self) -> Result<AlterDatabase> {
self.parser
.expect_keyword(Keyword::DATABASE)
.context(error::SyntaxSnafu)?;
let database_name = self
.parser
.parse_object_name(false)
.context(error::SyntaxSnafu)?;
let database_name = Self::canonicalize_object_name(database_name);
match self.parser.peek_token().token {
Token::Word(w) => {
if w.value.eq_ignore_ascii_case("UNSET") {
let _ = self.parser.next_token();
let keys = self
.parser
.parse_comma_separated(parse_string_option_names)
.context(error::SyntaxSnafu)?
.into_iter()
.map(|name| name.to_string())
.collect();
Ok(AlterDatabase::new(
database_name,
AlterDatabaseOperation::UnsetDatabaseOption { keys },
))
} else if w.keyword == 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)| KeyValueOption { key, value })
.collect();
Ok(AlterDatabase::new(
database_name,
AlterDatabaseOperation::SetDatabaseOption { options },
))
} else {
self.expected(
"SET or UNSET after ALTER DATABASE",
self.parser.peek_token(),
)
}
}
unexpected => self.unsupported(unexpected.to_string()),
}
}
fn parse_alter_table(&mut self) -> Result<AlterTable> {
self.parser
.expect_keywords(&[Keyword::ALTER, Keyword::TABLE])
.expect_keyword(Keyword::TABLE)
.context(error::SyntaxSnafu)?;
let raw_table_name = self
@@ -89,7 +148,7 @@ impl ParserContext<'_> {
.parse_comma_separated(parse_string_options)
.context(error::SyntaxSnafu)?
.into_iter()
.map(|(key, value)| TableOption { key, value })
.map(|(key, value)| KeyValueOption { key, value })
.collect();
AlterTableOperation::SetTableOptions { options }
}
@@ -261,6 +320,67 @@ mod tests {
use super::*;
use crate::dialect::GreptimeDbDialect;
use crate::parser::ParseOptions;
use crate::statements::alter::AlterDatabaseOperation;
#[test]
fn test_parse_alter_database() {
let sql = "ALTER DATABASE test_db SET 'a'='A', 'b' = 'B'";
let mut result =
ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
.unwrap();
assert_eq!(1, result.len());
let statement = result.remove(0);
assert_matches!(statement, Statement::AlterDatabase { .. });
match statement {
Statement::AlterDatabase(alter_database) => {
assert_eq!("test_db", alter_database.database_name().0[0].value);
let alter_operation = alter_database.alter_operation();
assert_matches!(
alter_operation,
AlterDatabaseOperation::SetDatabaseOption { .. }
);
match alter_operation {
AlterDatabaseOperation::SetDatabaseOption { options } => {
assert_eq!(2, options.len());
assert_eq!("a", options[0].key);
assert_eq!("A", options[0].value);
assert_eq!("b", options[1].key);
assert_eq!("B", options[1].value);
}
_ => unreachable!(),
}
}
_ => unreachable!(),
}
let sql = "ALTER DATABASE test_db UNSET 'a', 'b'";
let mut result =
ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
.unwrap();
assert_eq!(1, result.len());
let statement = result.remove(0);
assert_matches!(statement, Statement::AlterDatabase { .. });
match statement {
Statement::AlterDatabase(alter_database) => {
assert_eq!("test_db", alter_database.database_name().0[0].value);
let alter_operation = alter_database.alter_operation();
assert_matches!(
alter_operation,
AlterDatabaseOperation::UnsetDatabaseOption { .. }
);
match alter_operation {
AlterDatabaseOperation::UnsetDatabaseOption { keys } => {
assert_eq!(2, keys.len());
assert_eq!("a", keys[0]);
assert_eq!("b", keys[1]);
}
_ => unreachable!(),
}
}
_ => unreachable!(),
}
}
#[test]
fn test_parse_alter_add_column() {
@@ -271,9 +391,9 @@ mod tests {
assert_eq!(1, result.len());
let statement = result.remove(0);
assert_matches!(statement, Statement::Alter { .. });
assert_matches!(statement, Statement::AlterTable { .. });
match statement {
Statement::Alter(alter_table) => {
Statement::AlterTable(alter_table) => {
assert_eq!("my_metric_1", alter_table.table_name().0[0].value);
let alter_operation = alter_table.alter_operation();
@@ -307,9 +427,9 @@ mod tests {
assert_eq!(1, result.len());
let statement = result.remove(0);
assert_matches!(statement, Statement::Alter { .. });
assert_matches!(statement, Statement::AlterTable { .. });
match statement {
Statement::Alter(alter_table) => {
Statement::AlterTable(alter_table) => {
assert_eq!("my_metric_1", alter_table.table_name().0[0].value);
let alter_operation = alter_table.alter_operation();
@@ -343,9 +463,9 @@ mod tests {
assert_eq!(1, result.len());
let statement = result.remove(0);
assert_matches!(statement, Statement::Alter { .. });
assert_matches!(statement, Statement::AlterTable { .. });
match statement {
Statement::Alter(alter_table) => {
Statement::AlterTable(alter_table) => {
assert_eq!("my_metric_1", alter_table.table_name().0[0].value);
let alter_operation = alter_table.alter_operation();
@@ -394,9 +514,9 @@ mod tests {
assert_eq!(1, result.len());
let statement = result.remove(0);
assert_matches!(statement, Statement::Alter { .. });
assert_matches!(statement, Statement::AlterTable { .. });
match statement {
Statement::Alter(alter_table) => {
Statement::AlterTable(alter_table) => {
assert_eq!("my_metric_1", alter_table.table_name().0[0].value);
let alter_operation = alter_table.alter_operation();
@@ -433,9 +553,9 @@ mod tests {
assert_eq!(1, result_2.len());
let statement = result_2.remove(0);
assert_matches!(statement, Statement::Alter { .. });
assert_matches!(statement, Statement::AlterTable { .. });
match statement {
Statement::Alter(alter_table) => {
Statement::AlterTable(alter_table) => {
assert_eq!("my_metric_1", alter_table.table_name().0[0].value);
let alter_operation = alter_table.alter_operation();
@@ -469,7 +589,7 @@ mod tests {
.unwrap();
match result_1.remove(0) {
Statement::Alter(alter_table) => {
Statement::AlterTable(alter_table) => {
assert_eq!("my_metric_1", alter_table.table_name().0[0].value);
let alter_operation = alter_table.alter_operation();
@@ -500,7 +620,7 @@ mod tests {
.unwrap();
match result_2.remove(0) {
Statement::Alter(alter_table) => {
Statement::AlterTable(alter_table) => {
assert_eq!("my_metric_1", alter_table.table_name().0[0].value);
let alter_operation = alter_table.alter_operation();
@@ -539,9 +659,9 @@ mod tests {
assert_eq!(1, result.len());
let statement = result.remove(0);
assert_matches!(statement, Statement::Alter { .. });
assert_matches!(statement, Statement::AlterTable { .. });
match statement {
Statement::Alter(alter_table) => {
Statement::AlterTable(alter_table) => {
assert_eq!("test_table", alter_table.table_name().0[0].value);
let alter_operation = alter_table.alter_operation();
@@ -562,7 +682,7 @@ mod tests {
ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
.unwrap();
assert_eq!(1, result.len());
let Statement::Alter(alter) = &result[0] else {
let Statement::AlterTable(alter) = &result[0] else {
unreachable!()
};
assert_eq!("test_table", alter.table_name.0[0].value);
@@ -583,7 +703,7 @@ mod tests {
ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
.unwrap();
assert_eq!(1, result.len());
let Statement::Alter(alter) = &result[0] else {
let Statement::AlterTable(alter) = &result[0] else {
unreachable!()
};
assert_eq!("test_table", alter.table_name.0[0].value);
@@ -638,9 +758,9 @@ mod tests {
assert_eq!(1, result.len());
let statement = result.remove(0);
assert_matches!(statement, Statement::Alter { .. });
assert_matches!(statement, Statement::AlterTable { .. });
match statement {
Statement::Alter(alter_table) => {
Statement::AlterTable(alter_table) => {
assert_eq!("test_table", alter_table.table_name().0[0].value);
let alter_operation = alter_table.alter_operation();
@@ -675,9 +795,9 @@ mod tests {
.unwrap();
assert_eq!(1, result.len());
let statement = result.remove(0);
assert_matches!(statement, Statement::Alter { .. });
assert_matches!(statement, Statement::AlterTable { .. });
match statement {
Statement::Alter(alter_table) => {
Statement::AlterTable(alter_table) => {
assert_eq!("test_table", alter_table.table_name().0[0].value);
let alter_operation = alter_table.alter_operation();

View File

@@ -72,7 +72,7 @@ pub enum AlterTableOperation {
},
/// `SET <table attrs key> = <table attr value>`
SetTableOptions {
options: Vec<TableOption>,
options: Vec<KeyValueOption>,
},
UnsetTableOptions {
keys: Vec<String>,
@@ -123,7 +123,7 @@ impl Display for AlterTableOperation {
AlterTableOperation::SetTableOptions { options } => {
let kvs = options
.iter()
.map(|TableOption { key, value }| {
.map(|KeyValueOption { key, value }| {
if !value.is_empty() {
format!("'{key}'='{value}'")
} else {
@@ -152,20 +152,86 @@ impl Display for AlterTableOperation {
}
#[derive(Debug, Clone, PartialEq, Eq, Visit, VisitMut)]
pub struct TableOption {
pub struct KeyValueOption {
pub key: String,
pub value: String,
}
impl From<TableOption> for v1::TableOption {
fn from(c: TableOption) -> Self {
v1::TableOption {
impl From<KeyValueOption> for v1::Option {
fn from(c: KeyValueOption) -> Self {
v1::Option {
key: c.key,
value: c.value,
}
}
}
#[derive(Debug, Clone, PartialEq, Eq, Visit, VisitMut)]
pub struct AlterDatabase {
pub database_name: ObjectName,
pub alter_operation: AlterDatabaseOperation,
}
impl AlterDatabase {
pub(crate) fn new(database_name: ObjectName, alter_operation: AlterDatabaseOperation) -> Self {
Self {
database_name,
alter_operation,
}
}
pub fn database_name(&self) -> &ObjectName {
&self.database_name
}
pub fn alter_operation(&self) -> &AlterDatabaseOperation {
&self.alter_operation
}
}
impl Display for AlterDatabase {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let database_name = self.database_name();
let alter_operation = self.alter_operation();
write!(f, r#"ALTER DATABASE {database_name} {alter_operation}"#)
}
}
#[derive(Debug, Clone, PartialEq, Eq, Visit, VisitMut)]
pub enum AlterDatabaseOperation {
SetDatabaseOption { options: Vec<KeyValueOption> },
UnsetDatabaseOption { keys: Vec<String> },
}
impl Display for AlterDatabaseOperation {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
AlterDatabaseOperation::SetDatabaseOption { options } => {
let kvs = options
.iter()
.map(|KeyValueOption { key, value }| {
if !value.is_empty() {
format!("'{key}'='{value}'")
} else {
format!("'{key}'=NULL")
}
})
.join(",");
write!(f, "SET {kvs}")?;
Ok(())
}
AlterDatabaseOperation::UnsetDatabaseOption { keys } => {
let keys = keys.iter().map(|key| format!("'{key}'")).join(",");
write!(f, "UNSET {keys}")?;
Ok(())
}
}
}
}
#[cfg(test)]
mod tests {
use std::assert_matches::assert_matches;
@@ -176,15 +242,56 @@ mod tests {
#[test]
fn test_display_alter() {
let sql = r"ALTER DATABASE db SET 'a' = 'b', 'c' = 'd'";
let stmts =
ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
.unwrap();
assert_eq!(1, stmts.len());
assert_matches!(&stmts[0], Statement::AlterDatabase { .. });
match &stmts[0] {
Statement::AlterDatabase(set) => {
let new_sql = format!("\n{}", set);
assert_eq!(
r#"
ALTER DATABASE db SET 'a'='b','c'='d'"#,
&new_sql
);
}
_ => {
unreachable!();
}
}
let sql = r"ALTER DATABASE db UNSET 'a', 'c'";
let stmts =
ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
.unwrap();
assert_eq!(1, stmts.len());
match &stmts[0] {
Statement::AlterDatabase(set) => {
let new_sql = format!("\n{}", set);
assert_eq!(
r#"
ALTER DATABASE db UNSET 'a','c'"#,
&new_sql
);
}
_ => {
unreachable!();
}
}
let sql = r"alter table monitor add column app string default 'shop' primary key;";
let stmts =
ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
.unwrap();
assert_eq!(1, stmts.len());
assert_matches!(&stmts[0], Statement::Alter { .. });
assert_matches!(&stmts[0], Statement::AlterTable { .. });
match &stmts[0] {
Statement::Alter(set) => {
Statement::AlterTable(set) => {
let new_sql = format!("\n{}", set);
assert_eq!(
r#"
@@ -202,10 +309,10 @@ ALTER TABLE monitor ADD COLUMN app STRING DEFAULT 'shop' PRIMARY KEY"#,
ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
.unwrap();
assert_eq!(1, stmts.len());
assert_matches!(&stmts[0], Statement::Alter { .. });
assert_matches!(&stmts[0], Statement::AlterTable { .. });
match &stmts[0] {
Statement::Alter(set) => {
Statement::AlterTable(set) => {
let new_sql = format!("\n{}", set);
assert_eq!(
r#"
@@ -223,10 +330,10 @@ ALTER TABLE monitor MODIFY COLUMN load_15 STRING"#,
ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
.unwrap();
assert_eq!(1, stmts.len());
assert_matches!(&stmts[0], Statement::Alter { .. });
assert_matches!(&stmts[0], Statement::AlterTable { .. });
match &stmts[0] {
Statement::Alter(set) => {
Statement::AlterTable(set) => {
let new_sql = format!("\n{}", set);
assert_eq!(
r#"
@@ -244,10 +351,10 @@ ALTER TABLE monitor DROP COLUMN load_15"#,
ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
.unwrap();
assert_eq!(1, stmts.len());
assert_matches!(&stmts[0], Statement::Alter { .. });
assert_matches!(&stmts[0], Statement::AlterTable { .. });
match &stmts[0] {
Statement::Alter(set) => {
Statement::AlterTable(set) => {
let new_sql = format!("\n{}", set);
assert_eq!(
r#"
@@ -265,10 +372,10 @@ ALTER TABLE monitor RENAME monitor_new"#,
ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
.unwrap();
assert_eq!(1, stmts.len());
assert_matches!(&stmts[0], Statement::Alter { .. });
assert_matches!(&stmts[0], Statement::AlterTable { .. });
match &stmts[0] {
Statement::Alter(set) => {
Statement::AlterTable(set) => {
let new_sql = format!("\n{}", set);
assert_eq!(
r#"
@@ -286,10 +393,10 @@ ALTER TABLE monitor MODIFY COLUMN a SET FULLTEXT WITH(analyzer=English, case_sen
ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
.unwrap();
assert_eq!(1, stmts.len());
assert_matches!(&stmts[0], Statement::Alter { .. });
assert_matches!(&stmts[0], Statement::AlterTable { .. });
match &stmts[0] {
Statement::Alter(set) => {
Statement::AlterTable(set) => {
let new_sql = format!("\n{}", set);
assert_eq!(
r#"

View File

@@ -20,7 +20,7 @@ use sqlparser_derive::{Visit, VisitMut};
use crate::error::{ConvertToDfStatementSnafu, Error};
use crate::statements::admin::Admin;
use crate::statements::alter::AlterTable;
use crate::statements::alter::{AlterDatabase, AlterTable};
use crate::statements::create::{
CreateDatabase, CreateExternalTable, CreateFlow, CreateTable, CreateTableLike, CreateView,
};
@@ -70,7 +70,9 @@ pub enum Statement {
// CREATE DATABASE
CreateDatabase(CreateDatabase),
/// ALTER TABLE
Alter(AlterTable),
AlterTable(AlterTable),
/// ALTER DATABASE
AlterDatabase(AlterDatabase),
// Databases.
ShowDatabases(ShowDatabases),
// SHOW TABLES
@@ -133,7 +135,8 @@ impl Display for Statement {
Statement::DropDatabase(s) => s.fmt(f),
Statement::DropView(s) => s.fmt(f),
Statement::CreateDatabase(s) => s.fmt(f),
Statement::Alter(s) => s.fmt(f),
Statement::AlterTable(s) => s.fmt(f),
Statement::AlterDatabase(s) => s.fmt(f),
Statement::ShowDatabases(s) => s.fmt(f),
Statement::ShowTables(s) => s.fmt(f),
Statement::ShowTableStatus(s) => s.fmt(f),

View File

@@ -52,7 +52,7 @@ impl TransformRule for TypeAliasTransformRule {
.iter_mut()
.for_each(|column| replace_type_alias(column.mut_data_type()));
}
Statement::Alter(alter_table) => {
Statement::AlterTable(alter_table) => {
if let AlterTableOperation::ModifyColumnType { target_type, .. } =
alter_table.alter_operation_mut()
{