Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: create database with options #3751

Merged
merged 11 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions src/common/meta/src/ddl/create_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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 {
Expand Down Expand Up @@ -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?;
Expand Down Expand Up @@ -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>,
}
11 changes: 7 additions & 4 deletions src/common/meta/src/rpc/ddl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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>,
tisonkun marked this conversation as resolved.
Show resolved Hide resolved
}

impl TryFrom<PbCreateDatabaseTask> for CreateDatabaseTask {
Expand All @@ -665,7 +668,7 @@ impl TryFrom<PbCreateDatabaseTask> for CreateDatabaseTask {
catalog: catalog_name,
schema: schema_name,
create_if_not_exists,
options: Some(options),
options,
})
}
}
Expand All @@ -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,
}),
})
}
Expand Down
1 change: 1 addition & 0 deletions src/frontend/src/instance/grpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ impl GrpcQueryHandler for Instance {
.create_database(
&expr.schema_name,
expr.create_if_not_exists,
expr.options,
ctx.clone(),
)
.await?
Expand Down
1 change: 1 addition & 0 deletions src/operator/src/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ impl StatementExecutor {
self.create_database(
&format_raw_object_name(&stmt.name),
stmt.if_not_exists,
stmt.options.into_map(),
query_ctx,
)
.await
Expand Down
5 changes: 4 additions & 1 deletion src/operator/src/statement/ddl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,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();
Expand Down Expand Up @@ -827,6 +828,7 @@ impl StatementExecutor {
catalog.to_string(),
database.to_string(),
create_if_not_exists,
options,
query_context,
)
.await?;
Expand All @@ -844,11 +846,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
Expand Down
8 changes: 8 additions & 0 deletions src/sql/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 },

Expand Down Expand Up @@ -228,6 +235,7 @@ impl ErrorExt for Error {
InvalidColumnOption { .. }
| InvalidTableOptionValue { .. }
| InvalidDatabaseName { .. }
| InvalidDatabaseOption { .. }
| ColumnTypeMismatch { .. }
| InvalidTableName { .. }
| InvalidSqlValue { .. }
Expand Down
59 changes: 49 additions & 10 deletions src/sql/src/parsers/create_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand All @@ -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> {
Expand Down Expand Up @@ -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(),
}))
}

Expand Down Expand Up @@ -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]
Expand Down
35 changes: 33 additions & 2 deletions src/sql/src/statements/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
}
Expand All @@ -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(())
}
}

Expand Down Expand Up @@ -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]
Expand Down
13 changes: 11 additions & 2 deletions tests/cases/standalone/common/create/create_database.result
Original file line number Diff line number Diff line change
@@ -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: -
Expand All @@ -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;

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

6 changes: 5 additions & 1 deletion tests/cases/standalone/common/create/create_database.sql
Original file line number Diff line number Diff line change
@@ -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;