From 2e2dff0dec6ddf76d002cacde7b688551579495b Mon Sep 17 00:00:00 2001 From: ygf11 Date: Wed, 8 Jun 2022 05:52:13 +0000 Subject: [PATCH 1/2] feat(parser): fix display create database sql --- common/ast/src/ast/statement.rs | 6 ++++-- common/ast/src/parser/statement.rs | 4 ++-- common/ast/tests/it/testdata/statement.txt | 18 ++++++++++++------ query/src/sql/planner/binder/ddl/database.rs | 3 ++- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/common/ast/src/ast/statement.rs b/common/ast/src/ast/statement.rs index 1e485b4dd704..9a4b7c82ad32 100644 --- a/common/ast/src/ast/statement.rs +++ b/common/ast/src/ast/statement.rs @@ -206,7 +206,7 @@ pub struct CreateDatabaseStmt<'a> { pub if_not_exists: bool, pub catalog: Option>, pub database: Identifier<'a>, - pub engine: DatabaseEngine, + pub engine: Option, pub options: Vec, } @@ -473,7 +473,9 @@ impl<'a> Display for Statement<'a> { write!(f, "IF NOT EXISTS ")?; } write_period_separated_list(f, catalog.iter().chain(Some(database)))?; - write!(f, " ENGINE = {engine}")?; + if let Some(engine) = engine { + write!(f, " ENGINE = {engine}")?; + } // TODO(leiysky): display rest information } Statement::DropDatabase { diff --git a/common/ast/src/parser/statement.rs b/common/ast/src/parser/statement.rs index 636b4570131f..ac68031415f6 100644 --- a/common/ast/src/parser/statement.rs +++ b/common/ast/src/parser/statement.rs @@ -68,12 +68,12 @@ pub fn statement(i: Input) -> IResult { rule! { CREATE ~ ( DATABASE | SCHEMA ) ~ ( IF ~ NOT ~ EXISTS )? ~ ( #ident ~ "." )? ~ #ident ~ #database_engine? }, - |(_, _, opt_if_not_exists, opt_catalog, database, opt_engine)| { + |(_, _, opt_if_not_exists, opt_catalog, database, engine)| { Statement::CreateDatabase(CreateDatabaseStmt { if_not_exists: opt_if_not_exists.is_some(), catalog: opt_catalog.map(|(catalog, _)| catalog), database, - engine: opt_engine.unwrap_or(DatabaseEngine::Default), + engine, options: vec![], }) }, diff --git a/common/ast/tests/it/testdata/statement.txt b/common/ast/tests/it/testdata/statement.txt index 9bb85a00b934..ce74f5ddb479 100644 --- a/common/ast/tests/it/testdata/statement.txt +++ b/common/ast/tests/it/testdata/statement.txt @@ -508,7 +508,7 @@ UseDatabase { ---------- Input ---------- create database if not exists a; ---------- Output --------- -CREATE DATABASE IF NOT EXISTS a ENGINE = DEFAULT +CREATE DATABASE IF NOT EXISTS a ---------- AST ------------ CreateDatabase( CreateDatabaseStmt { @@ -519,7 +519,7 @@ CreateDatabase( quote: None, span: Ident(30..31), }, - engine: Default, + engine: None, options: [], }, ) @@ -545,7 +545,9 @@ CreateDatabase( quote: None, span: Ident(24..25), }, - engine: Default, + engine: Some( + Default, + ), options: [], }, ) @@ -565,8 +567,10 @@ CreateDatabase( quote: None, span: Ident(16..17), }, - engine: Github( - "123456", + engine: Some( + Github( + "123456", + ), ), options: [], }, @@ -587,7 +591,9 @@ CreateDatabase( quote: None, span: Ident(16..17), }, - engine: Default, + engine: Some( + Default, + ), options: [], }, ) diff --git a/query/src/sql/planner/binder/ddl/database.rs b/query/src/sql/planner/binder/ddl/database.rs index 83f2ec43f994..3daff0a82475 100644 --- a/query/src/sql/planner/binder/ddl/database.rs +++ b/query/src/sql/planner/binder/ddl/database.rs @@ -55,7 +55,8 @@ impl<'a> Binder { .map(|property| (property.name.clone(), property.value.clone())) .collect::>(); - let (engine, engine_options) = match &stmt.engine { + let database_engine = stmt.engine.as_ref().unwrap_or(&DatabaseEngine::Default); + let (engine, engine_options) = match database_engine { DatabaseEngine::Github(token) => { let engine_options = BTreeMap::from_iter(vec![("token".to_string(), token.clone())]); From 6ce5eb4294483e5815fe45cae9bb662308ef7d41 Mon Sep 17 00:00:00 2001 From: ygf11 Date: Wed, 8 Jun 2022 06:02:06 +0000 Subject: [PATCH 2/2] feat(planner): support drop database in new planner --- common/ast/src/ast/statement.rs | 25 ++++---- common/ast/src/parser/statement.rs | 11 ++-- common/ast/tests/it/parser.rs | 2 + common/ast/tests/it/testdata/statement.txt | 59 ++++++++++++++++--- .../interpreters/interpreter_factory_v2.rs | 5 ++ query/src/sql/optimizer/mod.rs | 1 + query/src/sql/planner/binder/ddl/database.rs | 24 ++++++++ query/src/sql/planner/binder/mod.rs | 4 ++ query/src/sql/planner/format/display_plan.rs | 1 + query/src/sql/planner/plans/mod.rs | 2 + .../05_0001_ddl_drop_database_v2.result | 0 .../05_ddl/05_0001_ddl_drop_database_v2.sql | 21 +++++++ 12 files changed, 134 insertions(+), 21 deletions(-) create mode 100644 tests/suites/0_stateless/05_ddl/05_0001_ddl_drop_database_v2.result create mode 100644 tests/suites/0_stateless/05_ddl/05_0001_ddl_drop_database_v2.sql diff --git a/common/ast/src/ast/statement.rs b/common/ast/src/ast/statement.rs index 9a4b7c82ad32..76a761865ddb 100644 --- a/common/ast/src/ast/statement.rs +++ b/common/ast/src/ast/statement.rs @@ -47,10 +47,7 @@ pub enum Statement<'a> { database: Identifier<'a>, }, CreateDatabase(CreateDatabaseStmt<'a>), - DropDatabase { - if_exists: bool, - database: Identifier<'a>, - }, + DropDatabase(DropDatabaseStmt<'a>), AlterDatabase { if_exists: bool, database: Identifier<'a>, @@ -210,6 +207,13 @@ pub struct CreateDatabaseStmt<'a> { pub options: Vec, } +#[derive(Debug, Clone, PartialEq)] +pub struct DropDatabaseStmt<'a> { + pub if_exists: bool, + pub catalog: Option>, + pub database: Identifier<'a>, +} + #[derive(Debug, Clone, PartialEq)] pub struct CreateTableStmt<'a> { pub if_not_exists: bool, @@ -478,15 +482,16 @@ impl<'a> Display for Statement<'a> { } // TODO(leiysky): display rest information } - Statement::DropDatabase { - database, + Statement::DropDatabase(DropDatabaseStmt { if_exists, - } => { - write!(f, "DROP DATABASE")?; + catalog, + database, + }) => { + write!(f, "DROP DATABASE ")?; if *if_exists { - write!(f, " IF EXISTS")?; + write!(f, "IF EXISTS ")?; } - write!(f, " {database}")?; + write_period_separated_list(f, catalog.iter().chain(Some(database)))?; } Statement::AlterDatabase { if_exists, diff --git a/common/ast/src/parser/statement.rs b/common/ast/src/parser/statement.rs index ac68031415f6..f46117d520ec 100644 --- a/common/ast/src/parser/statement.rs +++ b/common/ast/src/parser/statement.rs @@ -80,11 +80,14 @@ pub fn statement(i: Input) -> IResult { ); let drop_database = map( rule! { - DROP ~ ( DATABASE | SCHEMA ) ~ ( IF ~ EXISTS )? ~ #ident + DROP ~ ( DATABASE | SCHEMA ) ~ ( IF ~ EXISTS )? ~ ( #ident ~ "." )? ~ #ident }, - |(_, _, opt_if_exists, database)| Statement::DropDatabase { - if_exists: opt_if_exists.is_some(), - database, + |(_, _, opt_if_exists, opt_catalog, database)| { + Statement::DropDatabase(DropDatabaseStmt { + if_exists: opt_if_exists.is_some(), + catalog: opt_catalog.map(|(catalog, _)| catalog), + database, + }) }, ); let alter_database = map( diff --git a/common/ast/tests/it/parser.rs b/common/ast/tests/it/parser.rs index f1231453cf2e..05de2c339a0d 100644 --- a/common/ast/tests/it/parser.rs +++ b/common/ast/tests/it/parser.rs @@ -81,6 +81,8 @@ fn test_statement() { r#"create database catalog.t engine = Default;"#, r#"create database t engine = Github(token='123456');"#, r#"create database t engine = Default;"#, + r#"drop database catalog.t;"#, + r#"drop database if exists t;"#, r#"create table c(a DateTime null, b DateTime(3));"#, r#"create view v as select number % 3 as a from numbers(1000);"#, r#"truncate table test;"#, diff --git a/common/ast/tests/it/testdata/statement.txt b/common/ast/tests/it/testdata/statement.txt index ce74f5ddb479..f296a2750bf6 100644 --- a/common/ast/tests/it/testdata/statement.txt +++ b/common/ast/tests/it/testdata/statement.txt @@ -599,6 +599,48 @@ CreateDatabase( ) +---------- Input ---------- +drop database catalog.t; +---------- Output --------- +DROP DATABASE catalog.t +---------- AST ------------ +DropDatabase( + DropDatabaseStmt { + if_exists: false, + catalog: Some( + Identifier { + name: "catalog", + quote: None, + span: Ident(14..21), + }, + ), + database: Identifier { + name: "t", + quote: None, + span: Ident(22..23), + }, + }, +) + + +---------- Input ---------- +drop database if exists t; +---------- Output --------- +DROP DATABASE IF EXISTS t +---------- AST ------------ +DropDatabase( + DropDatabaseStmt { + if_exists: true, + catalog: None, + database: Identifier { + name: "t", + quote: None, + span: Ident(24..25), + }, + }, +) + + ---------- Input ---------- create table c(a DateTime null, b DateTime(3)); ---------- Output --------- @@ -1008,14 +1050,17 @@ DROP database if exists db1; ---------- Output --------- DROP DATABASE IF EXISTS db1 ---------- AST ------------ -DropDatabase { - if_exists: true, - database: Identifier { - name: "db1", - quote: None, - span: Ident(24..27), +DropDatabase( + DropDatabaseStmt { + if_exists: true, + catalog: None, + database: Identifier { + name: "db1", + quote: None, + span: Ident(24..27), + }, }, -} +) ---------- Input ---------- diff --git a/query/src/interpreters/interpreter_factory_v2.rs b/query/src/interpreters/interpreter_factory_v2.rs index 7762fae1cbc3..650f395efc05 100644 --- a/query/src/interpreters/interpreter_factory_v2.rs +++ b/query/src/interpreters/interpreter_factory_v2.rs @@ -19,6 +19,7 @@ use common_exception::Result; use super::CreateDatabaseInterpreter; use super::CreateTableInterpreter; use super::CreateViewInterpreter; +use super::DropDatabaseInterpreter; use super::ExplainInterpreterV2; use super::InterpreterPtr; use super::SelectInterpreterV2; @@ -50,6 +51,7 @@ impl InterpreterFactoryV2 { | DfStatement::ShowProcessList(_) | DfStatement::ShowSettings(_) | DfStatement::CreateDatabase(_) + | DfStatement::DropDatabase(_) ) } @@ -74,6 +76,9 @@ impl InterpreterFactoryV2 { Plan::CreateDatabase(create_database) => { CreateDatabaseInterpreter::try_create(ctx, create_database.clone()) } + Plan::DropDatabase(drop_database) => { + DropDatabaseInterpreter::try_create(ctx, drop_database.clone()) + } Plan::ShowMetrics => ShowMetricsInterpreter::try_create(ctx), Plan::ShowProcessList => ShowProcessListInterpreter::try_create(ctx), Plan::ShowSettings => ShowSettingsInterpreter::try_create(ctx), diff --git a/query/src/sql/optimizer/mod.rs b/query/src/sql/optimizer/mod.rs index a7082f085e7b..5bee7990bb1d 100644 --- a/query/src/sql/optimizer/mod.rs +++ b/query/src/sql/optimizer/mod.rs @@ -61,6 +61,7 @@ pub fn optimize(plan: Plan) -> Result { | Plan::ShowProcessList | Plan::ShowSettings | Plan::CreateDatabase(_) + | Plan::DropDatabase(_) | Plan::CreateTable(_) | Plan::CreateUser(_) | Plan::CreateView(_) diff --git a/query/src/sql/planner/binder/ddl/database.rs b/query/src/sql/planner/binder/ddl/database.rs index 3daff0a82475..34039059c4bb 100644 --- a/query/src/sql/planner/binder/ddl/database.rs +++ b/query/src/sql/planner/binder/ddl/database.rs @@ -16,14 +16,38 @@ use std::collections::BTreeMap; use common_ast::ast::CreateDatabaseStmt; use common_ast::ast::DatabaseEngine; +use common_ast::ast::DropDatabaseStmt; use common_exception::Result; use common_meta_app::schema::DatabaseMeta; use common_planners::CreateDatabasePlan; +use common_planners::DropDatabasePlan; use crate::sql::binder::Binder; use crate::sql::plans::Plan; impl<'a> Binder { + pub(in crate::sql::planner::binder) async fn bind_drop_database( + &self, + stmt: &DropDatabaseStmt<'a>, + ) -> Result { + let catalog = stmt + .catalog + .as_ref() + .map(|catalog| catalog.name.clone()) + .unwrap_or_else(|| self.ctx.get_current_catalog()); + + let tenant = self.ctx.get_tenant(); + let db = stmt.database.name.clone(); + let if_exists = stmt.if_exists; + + Ok(Plan::DropDatabase(DropDatabasePlan { + tenant, + catalog, + db, + if_exists, + })) + } + pub(in crate::sql::planner::binder) async fn bind_create_database( &self, stmt: &CreateDatabaseStmt<'a>, diff --git a/query/src/sql/planner/binder/mod.rs b/query/src/sql/planner/binder/mod.rs index f24c67a5d570..d27e21592c4e 100644 --- a/query/src/sql/planner/binder/mod.rs +++ b/query/src/sql/planner/binder/mod.rs @@ -114,6 +114,10 @@ impl<'a> Binder { let plan = self.bind_create_database(stmt).await?; Ok(plan) } + Statement::DropDatabase(stmt) => { + let plan = self.bind_drop_database(stmt).await?; + Ok(plan) + } Statement::ShowMetrics => Ok(Plan::ShowMetrics), Statement::ShowProcessList => Ok(Plan::ShowProcessList), diff --git a/query/src/sql/planner/format/display_plan.rs b/query/src/sql/planner/format/display_plan.rs index 3e7423fa7f00..6495c457b5ae 100644 --- a/query/src/sql/planner/format/display_plan.rs +++ b/query/src/sql/planner/format/display_plan.rs @@ -28,6 +28,7 @@ impl Plan { } Plan::CreateTable(create_table) => Ok(format!("{:?}", create_table)), Plan::CreateDatabase(create_database) => Ok(format!("{:?}", create_database)), + Plan::DropDatabase(drop_database) => Ok(format!("{:?}", drop_database)), Plan::ShowMetrics => Ok("SHOW METRICS".to_string()), Plan::ShowProcessList => Ok("SHOW PROCESSLIST".to_string()), Plan::ShowSettings => Ok("SHOW SETTINGS".to_string()), diff --git a/query/src/sql/planner/plans/mod.rs b/query/src/sql/planner/plans/mod.rs index de1f4e633d0c..56d102530202 100644 --- a/query/src/sql/planner/plans/mod.rs +++ b/query/src/sql/planner/plans/mod.rs @@ -36,6 +36,7 @@ use common_planners::CreateDatabasePlan; use common_planners::CreateTablePlan; use common_planners::CreateUserPlan; use common_planners::CreateViewPlan; +use common_planners::DropDatabasePlan; use common_planners::DropUserPlan; pub use eval_scalar::EvalScalar; pub use eval_scalar::ScalarItem; @@ -77,6 +78,7 @@ pub enum Plan { CreateTable(Box), CreateDatabase(CreateDatabasePlan), CreateView(Box), + DropDatabase(DropDatabasePlan), // System ShowMetrics, diff --git a/tests/suites/0_stateless/05_ddl/05_0001_ddl_drop_database_v2.result b/tests/suites/0_stateless/05_ddl/05_0001_ddl_drop_database_v2.result new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/suites/0_stateless/05_ddl/05_0001_ddl_drop_database_v2.sql b/tests/suites/0_stateless/05_ddl/05_0001_ddl_drop_database_v2.sql new file mode 100644 index 000000000000..c38f5d6db1b4 --- /dev/null +++ b/tests/suites/0_stateless/05_ddl/05_0001_ddl_drop_database_v2.sql @@ -0,0 +1,21 @@ +set enable_planner_v2 = 1; + +DROP DATABASE IF EXISTS db; + +CREATE DATABASE db; +DROP DATABASE catalog_not_exist.db; -- {ErrorCode 1006} +DROP DATABASE db; +DROP DATABASE IF EXISTS db; +DROP DATABASE db; -- {ErrorCode 1003} + +CREATE DATABASE db; +DROP DATABASE default.db; +DROP DATABASE IF EXISTS db; + +DROP SCHEMA IF EXISTS db; +CREATE SCHEMA db; +DROP SCHEMA db; +DROP SCHEMA IF EXISTS db; +DROP SCHEMA db; -- {ErrorCode 1003} + +set enable_planner_v2 = 0;