From 54067311859627ae04740652074068a8d3be8a8f Mon Sep 17 00:00:00 2001 From: Dennis Zhuang Date: Mon, 26 Dec 2022 20:11:54 +0800 Subject: [PATCH] feat: supports if_not_exists when create database --- src/api/greptime/v1/admin.proto | 1 + src/datanode/src/error.rs | 4 ++ src/datanode/src/instance/grpc.rs | 1 + src/datanode/src/instance/sql.rs | 1 + src/datanode/src/sql/create.rs | 16 ++++-- src/frontend/src/instance/distributed.rs | 1 + src/frontend/src/instance/prometheus.rs | 1 + src/sql/src/parsers/create_parser.rs | 20 ++++++- src/sql/src/statements/create.rs | 2 + src/table/src/requests.rs | 1 + tests/cases/standalone/catalog/schema.output | 50 +++++++++++++++++ tests/cases/standalone/catalog/schema.sql | 21 +++++++ tests/cases/standalone/create/create.result | 58 ++++++++++++++++++++ tests/cases/standalone/create/create.sql | 23 ++++++++ 14 files changed, 195 insertions(+), 5 deletions(-) create mode 100644 tests/cases/standalone/catalog/schema.output create mode 100644 tests/cases/standalone/catalog/schema.sql create mode 100644 tests/cases/standalone/create/create.result create mode 100644 tests/cases/standalone/create/create.sql diff --git a/src/api/greptime/v1/admin.proto b/src/api/greptime/v1/admin.proto index d5c9e93a7c96..2c4e62ca4452 100644 --- a/src/api/greptime/v1/admin.proto +++ b/src/api/greptime/v1/admin.proto @@ -64,6 +64,7 @@ message DropTableExpr { message CreateDatabaseExpr { //TODO(hl): maybe rename to schema_name? string database_name = 1; + bool create_if_not_exists = 2; } message AddColumns { diff --git a/src/datanode/src/error.rs b/src/datanode/src/error.rs index 6115bfa60aed..5d7a0823f8da 100644 --- a/src/datanode/src/error.rs +++ b/src/datanode/src/error.rs @@ -211,6 +211,9 @@ pub enum Error { source: catalog::error::Error, }, + #[snafu(display("Schema already exists, name: {}", name))] + SchemaExists { name: String, backtrace: Backtrace }, + #[snafu(display("Failed to decode as physical plan, source: {}", source))] IntoPhysicalPlan { #[snafu(backtrace)] @@ -341,6 +344,7 @@ impl ErrorExt for Error { | Error::CatalogNotFound { .. } | Error::SchemaNotFound { .. } | Error::ConstraintNotSupported { .. } + | Error::SchemaExists { .. } | Error::ParseTimestamp { .. } => StatusCode::InvalidArguments, // TODO(yingwen): Further categorize http error. diff --git a/src/datanode/src/instance/grpc.rs b/src/datanode/src/instance/grpc.rs index 39c53e9c26f0..6d49bddf0093 100644 --- a/src/datanode/src/instance/grpc.rs +++ b/src/datanode/src/instance/grpc.rs @@ -129,6 +129,7 @@ impl Instance { ) -> AdminResult { let req = CreateDatabaseRequest { db_name: create_database_expr.database_name, + create_if_not_exists: create_database_expr.create_if_not_exists, }; let result = self.sql_handler.create_database(req).await; match result { diff --git a/src/datanode/src/instance/sql.rs b/src/datanode/src/instance/sql.rs index f238a0374557..0d5767c29d67 100644 --- a/src/datanode/src/instance/sql.rs +++ b/src/datanode/src/instance/sql.rs @@ -65,6 +65,7 @@ impl Instance { Statement::CreateDatabase(c) => { let request = CreateDatabaseRequest { db_name: c.name.to_string(), + create_if_not_exists: c.if_not_exists, }; info!("Creating a new database: {}", request.db_name); diff --git a/src/datanode/src/sql/create.rs b/src/datanode/src/sql/create.rs index 7e83907e1b9b..3027d1412d84 100644 --- a/src/datanode/src/sql/create.rs +++ b/src/datanode/src/sql/create.rs @@ -33,22 +33,30 @@ use table::requests::*; use crate::error::{ self, CatalogNotFoundSnafu, CatalogSnafu, ConstraintNotSupportedSnafu, CreateSchemaSnafu, CreateTableSnafu, InsertSystemCatalogSnafu, InvalidPrimaryKeySnafu, KeyColumnNotFoundSnafu, - RegisterSchemaSnafu, Result, SchemaNotFoundSnafu, + RegisterSchemaSnafu, Result, SchemaExistsSnafu, SchemaNotFoundSnafu, }; use crate::sql::SqlHandler; impl SqlHandler { pub(crate) async fn create_database(&self, req: CreateDatabaseRequest) -> Result { let schema = req.db_name; - let req = RegisterSchemaRequest { + let reg_req = RegisterSchemaRequest { catalog: DEFAULT_CATALOG_NAME.to_string(), schema: schema.clone(), }; - self.catalog_manager - .register_schema(req) + let success = self + .catalog_manager + .register_schema(reg_req) .await .context(RegisterSchemaSnafu)?; + // FIXME(dennis): looks like register_schema always returns true even + // even when the schema already exists. + ensure!( + success || req.create_if_not_exists, + SchemaExistsSnafu { name: schema } + ); + info!("Successfully created database: {:?}", schema); Ok(Output::AffectedRows(1)) } diff --git a/src/frontend/src/instance/distributed.rs b/src/frontend/src/instance/distributed.rs index 31657c0f296c..43fc461c4452 100644 --- a/src/frontend/src/instance/distributed.rs +++ b/src/frontend/src/instance/distributed.rs @@ -158,6 +158,7 @@ impl DistInstance { Statement::CreateDatabase(stmt) => { let expr = CreateDatabaseExpr { database_name: stmt.name.to_string(), + create_if_not_exists: stmt.if_not_exists, }; self.handle_create_database(expr).await?; Ok(Output::AffectedRows(1)) diff --git a/src/frontend/src/instance/prometheus.rs b/src/frontend/src/instance/prometheus.rs index fde5e0b43cd2..8647cc646575 100644 --- a/src/frontend/src/instance/prometheus.rs +++ b/src/frontend/src/instance/prometheus.rs @@ -210,6 +210,7 @@ mod tests { instance .handle_create_database(CreateDatabaseExpr { database_name: db.to_string(), + create_if_not_exists: true, }) .await .unwrap(); diff --git a/src/sql/src/parsers/create_parser.rs b/src/sql/src/parsers/create_parser.rs index 8b046b23c28f..af9d9bb863a3 100644 --- a/src/sql/src/parsers/create_parser.rs +++ b/src/sql/src/parsers/create_parser.rs @@ -46,7 +46,7 @@ impl<'a> ParserContext<'a> { Token::Word(w) => match w.keyword { Keyword::TABLE => self.parse_create_table(), - Keyword::DATABASE => self.parse_create_database(), + Keyword::SCHEMA | Keyword::DATABASE => self.parse_create_database(), _ => self.unsupported(w.to_string()), }, @@ -57,6 +57,10 @@ impl<'a> ParserContext<'a> { fn parse_create_database(&mut self) -> Result { self.parser.next_token(); + let if_not_exists = + self.parser + .parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]); + let database_name = self .parser .parse_object_name() @@ -68,6 +72,7 @@ impl<'a> ParserContext<'a> { Ok(Statement::CreateDatabase(CreateDatabase { name: database_name, + if_not_exists, })) } @@ -583,6 +588,19 @@ mod tests { match &stmts[0] { Statement::CreateDatabase(c) => { assert_eq!(c.name.to_string(), "prometheus"); + assert!(!c.if_not_exists); + } + _ => unreachable!(), + } + + let sql = "create database if not exists prometheus"; + let stmts = ParserContext::create_with_dialect(sql, &GenericDialect {}).unwrap(); + + assert_eq!(1, stmts.len()); + match &stmts[0] { + Statement::CreateDatabase(c) => { + assert_eq!(c.name.to_string(), "prometheus"); + assert!(c.if_not_exists); } _ => unreachable!(), } diff --git a/src/sql/src/statements/create.rs b/src/sql/src/statements/create.rs index 01a7cb28cbbc..cfd7f33396bf 100644 --- a/src/sql/src/statements/create.rs +++ b/src/sql/src/statements/create.rs @@ -47,4 +47,6 @@ pub struct PartitionEntry { #[derive(Debug, PartialEq, Eq, Clone)] pub struct CreateDatabase { pub name: ObjectName, + /// Create if not exists + pub if_not_exists: bool, } diff --git a/src/table/src/requests.rs b/src/table/src/requests.rs index 9d5e877aad61..c690a20840a9 100644 --- a/src/table/src/requests.rs +++ b/src/table/src/requests.rs @@ -33,6 +33,7 @@ pub struct InsertRequest { #[derive(Debug, Clone)] pub struct CreateDatabaseRequest { pub db_name: String, + pub create_if_not_exists: bool, } /// Create table request diff --git a/tests/cases/standalone/catalog/schema.output b/tests/cases/standalone/catalog/schema.output new file mode 100644 index 000000000000..6394bcbee24b --- /dev/null +++ b/tests/cases/standalone/catalog/schema.output @@ -0,0 +1,50 @@ +CREATE SCHEMA test; + +MutateResult { success: 1, failure: 0 } + +CREATE TABLE test.hello(i BIGINT TIME INDEX); + +MutateResult { success: 1, failure: 0 } + +DROP TABLE test.hello; + +MutateResult { success: 1, failure: 0 } + +DROP SCHEMA test; + +Failed to execute, error: Datanode { code: 1001, msg: "Failed to execute sql, source: Cannot parse SQL, source: SQL statement is not supported: DROP SCHEMA test;, keyword: SCHEMA" } + +CREATE SCHEMA test; + +MutateResult { success: 1, failure: 0 } + +CREATE TABLE test.hello(i BIGINT TIME INDEX); + +MutateResult { success: 1, failure: 0 } + +INSERT INTO test.hello VALUES (2), (3), (4); + +MutateResult { success: 3, failure: 0 } + +SELECT * FROM test.hello; + ++-----------------------+ +| i, #Timestamp, #Int64 | ++-----------------------+ +| 2 | +| 3 | +| 4 | ++-----------------------+ + +DROP TABLE test.hello; + +MutateResult { success: 1, failure: 0 } + +DROP SCHEMA test; + +Failed to execute, error: Datanode { code: 1001, msg: "Failed to execute sql, source: Cannot parse SQL, source: SQL statement is not supported: DROP SCHEMA test;, keyword: SCHEMA" } + +SELECT * FROM test.hello; + +Failed to execute, error: Datanode { code: 3000, msg: "Failed to execute sql, source: Cannot plan SQL: SELECT * FROM test.hello, source: Error during planning: table 'greptime.test.hello' not found" } + diff --git a/tests/cases/standalone/catalog/schema.sql b/tests/cases/standalone/catalog/schema.sql new file mode 100644 index 000000000000..88add7fd0165 --- /dev/null +++ b/tests/cases/standalone/catalog/schema.sql @@ -0,0 +1,21 @@ +CREATE SCHEMA test; + +CREATE TABLE test.hello(i BIGINT TIME INDEX); + +DROP TABLE test.hello; + +DROP SCHEMA test; + +CREATE SCHEMA test; + +CREATE TABLE test.hello(i BIGINT TIME INDEX); + +INSERT INTO test.hello VALUES (2), (3), (4); + +SELECT * FROM test.hello; + +DROP TABLE test.hello; + +DROP SCHEMA test; + +SELECT * FROM test.hello; diff --git a/tests/cases/standalone/create/create.result b/tests/cases/standalone/create/create.result new file mode 100644 index 000000000000..553dbf88028c --- /dev/null +++ b/tests/cases/standalone/create/create.result @@ -0,0 +1,58 @@ +CREATE TABLE integers (i BIGINT TIME INDEX); + +MutateResult { success: 1, failure: 0 } + +CREATE TABLE IF NOT EXISTS integers (i BIGINT TIME INDEX); + +MutateResult { success: 1, failure: 0 } + +CREATE TABLE test1 (i INTEGER, j INTEGER); + +Failed to execute, error: Datanode { code: 1004, msg: "Missing timestamp column in request" } + +CREATE TABLE test1 (i INTEGER, j BIGINT TIME INDEX NOT NULL); + +MutateResult { success: 1, failure: 0 } + +CREATE TABLE test2 (i INTEGER, j BIGINT TIME INDEX NULL); + +Failed to execute, error: Datanode { code: 2000, msg: "Failed to execute sql, source: Cannot parse SQL, source: Unexpected token while parsing SQL statement: CREATE TABLE test2 (i INTEGER, j BIGINT TIME INDEX NULL);, expected: 'NOT NULL', found: NULL, source: sql parser error: Expected NOT, found: NULL" } + +CREATE TABLE test2 (i INTEGER, j BIGINT TIME INDEX); + +MutateResult { success: 1, failure: 0 } + +DESC TABLE INTEGERS; + +Failed to execute, error: Datanode { code: 1004, msg: "Failed to execute sql, source: Table not found: INTEGERS" } + +DESC TABLE test1; + ++------------------------+-----------------------+-----------------------+--------------------------+--------------------------------+ +| Field, #Field, #String | Type, #Field, #String | Null, #Field, #String | Default, #Field, #String | Semantic Type, #Field, #String | ++------------------------+-----------------------+-----------------------+--------------------------+--------------------------------+ +| i | Int32 | YES | | VALUE | +| j | Int64 | NO | | TIME INDEX | ++------------------------+-----------------------+-----------------------+--------------------------+--------------------------------+ + +DESC TABLE test2; + ++------------------------+-----------------------+-----------------------+--------------------------+--------------------------------+ +| Field, #Field, #String | Type, #Field, #String | Null, #Field, #String | Default, #Field, #String | Semantic Type, #Field, #String | ++------------------------+-----------------------+-----------------------+--------------------------+--------------------------------+ +| i | Int32 | YES | | VALUE | +| j | Int64 | NO | | TIME INDEX | ++------------------------+-----------------------+-----------------------+--------------------------+--------------------------------+ + +DROP TABLE integers; + +MutateResult { success: 1, failure: 0 } + +DROP TABLE test1; + +MutateResult { success: 1, failure: 0 } + +DROP TABLE test2; + +MutateResult { success: 1, failure: 0 } + diff --git a/tests/cases/standalone/create/create.sql b/tests/cases/standalone/create/create.sql new file mode 100644 index 000000000000..62c10fc0ecff --- /dev/null +++ b/tests/cases/standalone/create/create.sql @@ -0,0 +1,23 @@ +CREATE TABLE integers (i BIGINT TIME INDEX); + +CREATE TABLE IF NOT EXISTS integers (i BIGINT TIME INDEX); + +CREATE TABLE test1 (i INTEGER, j INTEGER); + +CREATE TABLE test1 (i INTEGER, j BIGINT TIME INDEX NOT NULL); + +CREATE TABLE test2 (i INTEGER, j BIGINT TIME INDEX NULL); + +CREATE TABLE test2 (i INTEGER, j BIGINT TIME INDEX); + +DESC TABLE INTEGERS; + +DESC TABLE test1; + +DESC TABLE test2; + +DROP TABLE integers; + +DROP TABLE test1; + +DROP TABLE test2;