feat: create database with options (#3751)

* feat: create database with options * fix: clippy * fix: clippy * feat: rebase and add Display test * feat: sqlness test for creating database with options * address comments Signed-off-by: tison <wander4096@gmail.com> * fixup tests Signed-off-by: tison <wander4096@gmail.com> * catch up Signed-off-by: tison <wander4096@gmail.com> * DefaultOnNull Signed-off-by: tison <wander4096@gmail.com> --------- Signed-off-by: tison <wander4096@gmail.com> Co-authored-by: tison <wander4096@gmail.com>
GreptimeTeam · May 13, 2024 · 9d12496 · 9d12496
1 parent 5d8084a
commit 9d12496
Show file tree

Hide file tree

Showing 10 changed files with 126 additions and 29 deletions.
diff --git a/src/common/meta/src/ddl/create_database.rs b/src/common/meta/src/ddl/create_database.rs
@@ -18,6 +18,7 @@ use async_trait::async_trait;
 use common_procedure::error::{FromJsonSnafu, Result as ProcedureResult, ToJsonSnafu};
 use common_procedure::{Context as ProcedureContext, LockKey, Procedure, Status};
 use serde::{Deserialize, Serialize};
+use serde_with::{serde_as, DefaultOnNull};
 use snafu::{ensure, ResultExt};
 use strum::AsRefStr;
 
@@ -39,7 +40,7 @@ impl CreateDatabaseProcedure {
         catalog: String,
         schema: String,
         create_if_not_exists: bool,
-        options: Option<HashMap<String, String>>,
+        options: HashMap<String, String>,
         context: DdlContext,
     ) -> Self {
         Self {
@@ -85,19 +86,14 @@ impl CreateDatabaseProcedure {
     }
 
     pub async fn on_create_metadata(&mut self) -> Result<Status> {
-        let value: Option<SchemaNameValue> = self
-            .data
-            .options
-            .as_ref()
-            .map(|hash_map_ref| hash_map_ref.try_into())
-            .transpose()?;
+        let value: SchemaNameValue = (&self.data.options).try_into()?;
 
         self.context
             .table_metadata_manager
             .schema_manager()
             .create(
                 SchemaNameKey::new(&self.data.catalog, &self.data.schema),
-                value,
+                Some(value),
                 self.data.create_if_not_exists,
             )
             .await?;
@@ -142,11 +138,13 @@ pub enum CreateDatabaseState {
     CreateMetadata,
 }
 
+#[serde_as]
 #[derive(Debug, Serialize, Deserialize)]
 pub struct CreateDatabaseData {
     pub state: CreateDatabaseState,
     pub catalog: String,
     pub schema: String,
     pub create_if_not_exists: bool,
-    pub options: Option<HashMap<String, String>>,
+    #[serde_as(deserialize_as = "DefaultOnNull")]
+    pub options: HashMap<String, String>,
 }
diff --git a/src/common/meta/src/rpc/ddl.rs b/src/common/meta/src/rpc/ddl.rs
@@ -33,6 +33,7 @@ use base64::engine::general_purpose;
 use base64::Engine as _;
 use prost::Message;
 use serde::{Deserialize, Serialize};
+use serde_with::{serde_as, DefaultOnNull};
 use session::context::QueryContextRef;
 use snafu::{OptionExt, ResultExt};
 use table::metadata::{RawTableInfo, TableId};
@@ -112,7 +113,7 @@ impl DdlTask {
         catalog: String,
         schema: String,
         create_if_not_exists: bool,
-        options: Option<HashMap<String, String>>,
+        options: HashMap<String, String>,
     ) -> Self {
         DdlTask::CreateDatabase(CreateDatabaseTask {
             catalog,
@@ -640,12 +641,14 @@ impl TryFrom<TruncateTableTask> for PbTruncateTableTask {
     }
 }
 
+#[serde_as]
 #[derive(Debug, PartialEq, Serialize, Deserialize, Clone)]
 pub struct CreateDatabaseTask {
     pub catalog: String,
     pub schema: String,
     pub create_if_not_exists: bool,
-    pub options: Option<HashMap<String, String>>,
+    #[serde_as(deserialize_as = "DefaultOnNull")]
+    pub options: HashMap<String, String>,
 }
 
 impl TryFrom<PbCreateDatabaseTask> for CreateDatabaseTask {
@@ -665,7 +668,7 @@ impl TryFrom<PbCreateDatabaseTask> for CreateDatabaseTask {
             catalog: catalog_name,
             schema: schema_name,
             create_if_not_exists,
-            options: Some(options),
+            options,
         })
     }
 }
@@ -686,7 +689,7 @@ impl TryFrom<CreateDatabaseTask> for PbCreateDatabaseTask {
                 catalog_name: catalog,
                 schema_name: schema,
                 create_if_not_exists,
-                options: options.unwrap_or_default(),
+                options,
             }),
         })
     }

diff --git a/src/frontend/src/instance/grpc.rs b/src/frontend/src/instance/grpc.rs
@@ -125,6 +125,7 @@ impl GrpcQueryHandler for Instance {
                             .create_database(
                                 &expr.schema_name,
                                 expr.create_if_not_exists,
+                                expr.options,
                                 ctx.clone(),
                             )
                             .await?

diff --git a/src/operator/src/statement.rs b/src/operator/src/statement.rs
@@ -208,6 +208,7 @@ impl StatementExecutor {
                 self.create_database(
                     &format_raw_object_name(&stmt.name),
                     stmt.if_not_exists,
+                    stmt.options.into_map(),
                     query_ctx,
                 )
                 .await

diff --git a/src/operator/src/statement/ddl.rs b/src/operator/src/statement/ddl.rs
@@ -813,6 +813,7 @@ impl StatementExecutor {
         &self,
         database: &str,
         create_if_not_exists: bool,
+        options: HashMap<String, String>,
         query_context: QueryContextRef,
     ) -> Result<Output> {
         let catalog = query_context.current_catalog();
@@ -840,6 +841,7 @@ impl StatementExecutor {
                 catalog.to_string(),
                 database.to_string(),
                 create_if_not_exists,
+                options,
                 query_context,
             )
             .await?;
@@ -857,11 +859,12 @@ impl StatementExecutor {
         catalog: String,
         database: String,
         create_if_not_exists: bool,
+        options: HashMap<String, String>,
         query_context: QueryContextRef,
     ) -> Result<SubmitDdlTaskResponse> {
         let request = SubmitDdlTaskRequest {
             query_context,
-            task: DdlTask::new_create_database(catalog, database, create_if_not_exists, None),
+            task: DdlTask::new_create_database(catalog, database, create_if_not_exists, options),
         };
 
         self.procedure_executor

diff --git a/src/sql/src/error.rs b/src/sql/src/error.rs
@@ -123,6 +123,13 @@ pub enum Error {
     #[snafu(display("Invalid database name: {}", name))]
     InvalidDatabaseName { name: String },
 
+    #[snafu(display("Unrecognized database option key: {}", key))]
+    InvalidDatabaseOption {
+        key: String,
+        #[snafu(implicit)]
+        location: Location,
+    },
+
     #[snafu(display("Invalid table name: {}", name))]
     InvalidTableName { name: String },
 
@@ -228,6 +235,7 @@ impl ErrorExt for Error {
             InvalidColumnOption { .. }
             | InvalidTableOptionValue { .. }
             | InvalidDatabaseName { .. }
+            | InvalidDatabaseOption { .. }
             | ColumnTypeMismatch { .. }
             | InvalidTableName { .. }
             | InvalidSqlValue { .. }

diff --git a/src/sql/src/parsers/create_parser.rs b/src/sql/src/parsers/create_parser.rs
@@ -27,8 +27,9 @@ use table::requests::validate_table_option;
 
 use crate::ast::{ColumnDef, Ident, TableConstraint};
 use crate::error::{
-    self, InvalidColumnOptionSnafu, InvalidTableOptionSnafu, InvalidTimeIndexSnafu,
-    MissingTimeIndexSnafu, Result, SyntaxSnafu, UnexpectedSnafu, UnsupportedSnafu,
+    self, InvalidColumnOptionSnafu, InvalidDatabaseOptionSnafu, InvalidTableOptionSnafu,
+    InvalidTimeIndexSnafu, MissingTimeIndexSnafu, Result, SyntaxSnafu, UnexpectedSnafu,
+    UnsupportedSnafu,
 };
 use crate::parser::{ParserContext, FLOW};
 use crate::statements::create::{
@@ -45,6 +46,12 @@ pub const SINK: &str = "SINK";
 pub const EXPIRE: &str = "EXPIRE";
 pub const WHEN: &str = "WHEN";
 
+const DB_OPT_KEY_TTL: &str = "ttl";
+
+fn validate_database_option(key: &str) -> bool {
+    [DB_OPT_KEY_TTL].contains(&key)
+}
+
 /// Parses create [table] statement
 impl<'a> ParserContext<'a> {
     pub(crate) fn parse_create(&mut self) -> Result<Statement> {
@@ -124,9 +131,28 @@ impl<'a> ParserContext<'a> {
             actual: self.peek_token_as_string(),
         })?;
         let database_name = Self::canonicalize_object_name(database_name);
+
+        let options = self
+            .parser
+            .parse_options(Keyword::WITH)
+            .context(SyntaxSnafu)?
+            .into_iter()
+            .map(parse_option_string)
+            .collect::<Result<HashMap<String, String>>>()?;
+
+        for key in options.keys() {
+            ensure!(
+                validate_database_option(key),
+                InvalidDatabaseOptionSnafu {
+                    key: key.to_string()
+                }
+            );
+        }
+
         Ok(Statement::CreateDatabase(CreateDatabase {
             name: database_name,
             if_not_exists,
+            options: options.into(),
         }))
     }
 
@@ -1025,14 +1051,27 @@ mod tests {
         let sql = "CREATE DATABASE `fOo`";
         let result =
             ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default());
-        let mut stmts = result.unwrap();
-        assert_eq!(
-            stmts.pop().unwrap(),
-            Statement::CreateDatabase(CreateDatabase::new(
-                ObjectName(vec![Ident::with_quote('`', "fOo"),]),
-                false
-            ))
-        );
+        let stmts = result.unwrap();
+        match &stmts.last().unwrap() {
+            Statement::CreateDatabase(c) => {
+                assert_eq!(c.name, ObjectName(vec![Ident::with_quote('`', "fOo")]));
+                assert!(!c.if_not_exists);
+            }
+            _ => unreachable!(),
+        }
+
+        let sql = "CREATE DATABASE prometheus with (ttl='1h');";
+        let result =
+            ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default());
+        let stmts = result.unwrap();
+        match &stmts[0] {
+            Statement::CreateDatabase(c) => {
+                assert_eq!(c.name.to_string(), "prometheus");
+                assert!(!c.if_not_exists);
+                assert_eq!(c.options.get("ttl").unwrap(), "1h");
+            }
+            _ => unreachable!(),
+        }
     }
 
     #[test]

diff --git a/src/sql/src/statements/create.rs b/src/sql/src/statements/create.rs
@@ -168,14 +168,16 @@ pub struct CreateDatabase {
     pub name: ObjectName,
     /// Create if not exists
     pub if_not_exists: bool,
+    pub options: OptionMap,
 }
 
 impl CreateDatabase {
     /// Creates a statement for `CREATE DATABASE`
-    pub fn new(name: ObjectName, if_not_exists: bool) -> Self {
+    pub fn new(name: ObjectName, if_not_exists: bool, options: OptionMap) -> Self {
         Self {
             name,
             if_not_exists,
+            options,
         }
     }
 }
@@ -186,7 +188,12 @@ impl Display for CreateDatabase {
         if self.if_not_exists {
             write!(f, "IF NOT EXISTS ")?;
         }
-        write!(f, "{}", &self.name)
+        write!(f, "{}", &self.name)?;
+        if !self.options.is_empty() {
+            let options = self.options.kv_pairs();
+            write!(f, "\nWITH(\n{}\n)", format_list_indent!(options))?;
+        }
+        Ok(())
     }
 }
 
@@ -475,6 +482,30 @@ CREATE DATABASE IF NOT EXISTS test"#,
                 unreachable!();
             }
         }
+
+        let sql = r#"CREATE DATABASE IF NOT EXISTS test WITH (ttl='1h');"#;
+        let stmts =
+            ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
+                .unwrap();
+        assert_eq!(1, stmts.len());
+        assert_matches!(&stmts[0], Statement::CreateDatabase { .. });
+
+        match &stmts[0] {
+            Statement::CreateDatabase(set) => {
+                let new_sql = format!("\n{}", set);
+                assert_eq!(
+                    r#"
+CREATE DATABASE IF NOT EXISTS test
+WITH(
+  ttl = '1h'
+)"#,
+                    &new_sql
+                );
+            }
+            _ => {
+                unreachable!();
+            }
+        }
     }
 
     #[test]

diff --git a/tests/cases/standalone/common/create/create_database.result b/tests/cases/standalone/common/create/create_database.result
@@ -1,3 +1,7 @@
+create database '㊙️database';
+
+Error: 1002(Unexpected), Unexpected, violated: Invalid database name: ㊙️database
+
 create database illegal-database;
 
 Error: 1001(Unsupported), SQL statement is not supported: create database illegal-database;, keyword: -
@@ -6,9 +10,9 @@ create database 'illegal-database';
 
 Affected Rows: 1
 
-create database '㊙️database';
+create database mydb with (ttl = '1h');
 
-Error: 1002(Unexpected), Unexpected, violated: Invalid database name: ㊙️database
+Affected Rows: 1
 
 show databases;
 
@@ -18,10 +22,15 @@ show databases;
 | greptime_private   |
 | illegal-database   |
 | information_schema |
+| mydb               |
 | public             |
 +--------------------+
 
 drop database 'illegal-database';
 
 Affected Rows: 0
 
+drop database mydb;
+
+Affected Rows: 0
+
diff --git a/tests/cases/standalone/common/create/create_database.sql b/tests/cases/standalone/common/create/create_database.sql
@@ -1,9 +1,13 @@
+create database '㊙️database';
+
 create database illegal-database;
 
 create database 'illegal-database';
 
-create database '㊙️database';
+create database mydb with (ttl = '1h');
 
 show databases;
 
 drop database 'illegal-database';
+
+drop database mydb;