From c53c105f831d4b6ad585e83107e328749136542a Mon Sep 17 00:00:00 2001 From: kould Date: Sat, 24 Feb 2024 02:32:29 +0800 Subject: [PATCH 1/5] feat: support `Create Table ... Like` --- src/frontend/src/instance.rs | 6 +- src/operator/src/expr_factory.rs | 2 +- src/operator/src/statement.rs | 6 +- src/operator/src/statement/ddl.rs | 45 ++++++++++++++ src/operator/src/statement/show.rs | 2 +- src/query/src/sql.rs | 11 +--- src/session/src/context.rs | 11 ++++ src/sql/src/parsers/create_parser.rs | 58 +++++++++++++------ src/sql/src/statements/create.rs | 7 +++ src/sql/src/statements/statement.rs | 6 +- .../standalone/common/create/create.result | 38 ++++++++++++ .../cases/standalone/common/create/create.sql | 14 +++++ 12 files changed, 172 insertions(+), 34 deletions(-) diff --git a/src/frontend/src/instance.rs b/src/frontend/src/instance.rs index 830261bca9af..47757104e855 100644 --- a/src/frontend/src/instance.rs +++ b/src/frontend/src/instance.rs @@ -460,8 +460,10 @@ pub fn check_permission( // database ops won't be checked Statement::CreateDatabase(_) | Statement::ShowDatabases(_) => {} // show create table and alter are not supported yet - Statement::ShowCreateTable(_) | Statement::CreateExternalTable(_) | Statement::Alter(_) => { - } + Statement::ShowCreateTable(_) + | Statement::CreateExternalTable(_) + | Statement::CreateTableLike(_) + | Statement::Alter(_) => {} // set/show variable now only alter/show variable in session Statement::SetVariables(_) | Statement::ShowVariables(_) => {} diff --git a/src/operator/src/expr_factory.rs b/src/operator/src/expr_factory.rs index 9f205ad9ed7c..0ad8e65828ff 100644 --- a/src/operator/src/expr_factory.rs +++ b/src/operator/src/expr_factory.rs @@ -181,7 +181,7 @@ pub(crate) async fn create_external_expr( Ok(expr) } -/// Convert `CreateTable` statement to `CreateExpr` gRPC request. +/// Convert `CreateTable` statement to `CreateTableExpr` gRPC request. pub fn create_to_expr(create: &CreateTable, query_ctx: QueryContextRef) -> Result { let (catalog_name, schema_name, table_name) = table_idents_to_full_name(&create.name, &query_ctx) diff --git a/src/operator/src/statement.rs b/src/operator/src/statement.rs index 1361fbb12ce7..42a44372fbad 100644 --- a/src/operator/src/statement.rs +++ b/src/operator/src/statement.rs @@ -153,6 +153,10 @@ impl StatementExecutor { let _ = self.create_table(stmt, query_ctx).await?; Ok(Output::AffectedRows(0)) } + Statement::CreateTableLike(stmt) => { + let _ = self.create_table_like(stmt, query_ctx).await?; + Ok(Output::AffectedRows(0)) + } Statement::CreateExternalTable(stmt) => { let _ = self.create_external_table(stmt, query_ctx).await?; Ok(Output::AffectedRows(0)) @@ -174,7 +178,6 @@ impl StatementExecutor { let table_name = TableName::new(catalog, schema, table); self.truncate_table(table_name).await } - Statement::CreateDatabase(stmt) => { self.create_database( query_ctx.current_catalog(), @@ -183,7 +186,6 @@ impl StatementExecutor { ) .await } - Statement::ShowCreateTable(show) => { let (catalog, schema, table) = table_idents_to_full_name(&show.table_name, &query_ctx) diff --git a/src/operator/src/statement/ddl.rs b/src/operator/src/statement/ddl.rs index b8b3b4eabaa8..86cd9a97ded9 100644 --- a/src/operator/src/statement/ddl.rs +++ b/src/operator/src/statement/ddl.rs @@ -21,6 +21,7 @@ use catalog::CatalogManagerRef; use chrono::Utc; use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; use common_catalog::format_full_table_name; +use common_error::ext::BoxedError; use common_meta::cache_invalidator::Context; use common_meta::ddl::ExecutorContext; use common_meta::key::schema_name::{SchemaNameKey, SchemaNameValue}; @@ -34,6 +35,7 @@ use datatypes::prelude::ConcreteDataType; use datatypes::schema::RawSchema; use lazy_static::lazy_static; use partition::partition::{PartitionBound, PartitionDef}; +use query::sql::show_create_table; use regex::Regex; use session::context::QueryContextRef; use snafu::{ensure, IntoError, OptionExt, ResultExt}; @@ -54,6 +56,8 @@ use crate::error::{ UnrecognizedTableOptionSnafu, }; use crate::expr_factory; +use crate::statement::show::create_partitions_stmt; +use crate::table::table_idents_to_full_name; lazy_static! { static ref NAME_PATTERN_REG: Regex = Regex::new(&format!("^{NAME_PATTERN}$")).unwrap(); @@ -71,6 +75,47 @@ impl StatementExecutor { .await } + #[tracing::instrument(skip_all)] + pub async fn create_table_like( + &self, + stmt: CreateTableLike, + ctx: QueryContextRef, + ) -> Result { + let (catalog, schema, table) = table_idents_to_full_name(&stmt.target, &ctx) + .map_err(BoxedError::new) + .context(error::ExternalSnafu)?; + let table_ref = self + .catalog_manager + .table(&catalog, &schema, &table) + .await + .context(error::CatalogSnafu)? + .context(error::TableNotFoundSnafu { table_name: &table })?; + let partitions = self + .partition_manager + .find_table_partitions(table_ref.table_info().table_id()) + .await + .context(error::FindTablePartitionRuleSnafu { table_name: table })?; + + let quote_style = ctx.quote_style(); + let mut create_stmt = + show_create_table::create_table_stmt(&table_ref.table_info(), quote_style) + .context(error::ParseQuerySnafu)?; + create_stmt.name = stmt.name; + create_stmt.if_not_exists = false; + + let partitions = create_partitions_stmt(partitions)?.and_then(|mut partitions| { + if !partitions.column_list.is_empty() { + partitions.set_quote(quote_style); + Some(partitions) + } else { + None + } + }); + + let create_expr = &mut expr_factory::create_to_expr(&create_stmt, ctx.clone())?; + self.create_table_inner(create_expr, partitions, &ctx).await + } + #[tracing::instrument(skip_all)] pub async fn create_external_table( &self, diff --git a/src/operator/src/statement/show.rs b/src/operator/src/statement/show.rs index 8f11e2b2340b..d73737cc967b 100644 --- a/src/operator/src/statement/show.rs +++ b/src/operator/src/statement/show.rs @@ -76,7 +76,7 @@ impl StatementExecutor { } } -fn create_partitions_stmt(partitions: Vec) -> Result> { +pub(crate) fn create_partitions_stmt(partitions: Vec) -> Result> { if partitions.is_empty() { return Ok(None); } diff --git a/src/query/src/sql.rs b/src/query/src/sql.rs index 34ff7fbe8a82..e170f1208473 100644 --- a/src/query/src/sql.rs +++ b/src/query/src/sql.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -mod show_create_table; +pub mod show_create_table; use std::collections::HashMap; use std::sync::Arc; @@ -258,14 +258,7 @@ pub fn show_create_table( let table_info = table.table_info(); let table_name = &table_info.name; - // Default to double quote and fallback to back quote - let quote_style = if query_ctx.sql_dialect().is_delimited_identifier_start('"') { - '"' - } else if query_ctx.sql_dialect().is_delimited_identifier_start('\'') { - '\'' - } else { - '`' - }; + let quote_style = query_ctx.quote_style(); let mut stmt = show_create_table::create_table_stmt(&table_info, quote_style)?; stmt.partitions = partitions.map(|mut p| { diff --git a/src/session/src/context.rs b/src/session/src/context.rs index 53cc2db7441a..cc41af37445b 100644 --- a/src/session/src/context.rs +++ b/src/session/src/context.rs @@ -172,6 +172,17 @@ impl QueryContext { session.set_timezone(tz.as_ref().clone()) } } + + /// Default to double quote and fallback to back quote + pub fn quote_style(&self) -> char { + if self.sql_dialect().is_delimited_identifier_start('"') { + '"' + } else if self.sql_dialect().is_delimited_identifier_start('\'') { + '\'' + } else { + '`' + } + } } impl QueryContextBuilder { diff --git a/src/sql/src/parsers/create_parser.rs b/src/sql/src/parsers/create_parser.rs index d47de7f862ce..63759e0419ba 100644 --- a/src/sql/src/parsers/create_parser.rs +++ b/src/sql/src/parsers/create_parser.rs @@ -66,15 +66,7 @@ impl<'a> ParserContext<'a> { let if_not_exists = self.parser .parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]); - let raw_table_name = self - .parser - .parse_object_name() - .context(error::UnexpectedSnafu { - sql: self.sql, - expected: "a table name", - actual: self.peek_token_as_string(), - })?; - let table_name = Self::canonicalize_object_name(raw_table_name); + let table_name = self.parse_table_name()?; let (columns, constraints) = self.parse_columns()?; let engine = self.parse_table_engine(common_catalog::consts::FILE_ENGINE)?; let options = self @@ -136,15 +128,16 @@ impl<'a> ParserContext<'a> { self.parser .parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]); - let raw_table_name = self - .parser - .parse_object_name() - .context(error::UnexpectedSnafu { - sql: self.sql, - expected: "a table name", - actual: self.peek_token_as_string(), - })?; - let table_name = Self::canonicalize_object_name(raw_table_name); + let table_name = self.parse_table_name()?; + + if self.parser.parse_keyword(Keyword::LIKE) { + let target_table_name = self.parse_table_name()?; + + return Ok(Statement::CreateTableLike(CreateTableLike { + name: table_name, + target: target_table_name, + })); + } let (columns, constraints) = self.parse_columns()?; @@ -180,6 +173,18 @@ impl<'a> ParserContext<'a> { Ok(Statement::CreateTable(create_table)) } + fn parse_table_name(&mut self) -> Result { + let raw_table_name = self + .parser + .parse_object_name() + .context(error::UnexpectedSnafu { + sql: self.sql, + expected: "a table name", + actual: self.peek_token_as_string(), + })?; + Ok(Self::canonicalize_object_name(raw_table_name)) + } + /// "PARTITION BY ..." syntax: // TODO(ruihang): docs fn parse_partitions(&mut self) -> Result> { @@ -739,6 +744,23 @@ mod tests { use crate::dialect::GreptimeDbDialect; use crate::parser::ParseOptions; + #[test] + fn test_parse_create_table_like() { + let sql = "CREATE TABLE t1 LIKE t2"; + let stmts = + ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default()) + .unwrap(); + + assert_eq!(1, stmts.len()); + match &stmts[0] { + Statement::CreateTableLike(c) => { + assert_eq!(c.name.to_string(), "t1"); + assert_eq!(c.target.to_string(), "t2"); + } + _ => unreachable!(), + } + } + #[test] fn test_validate_external_table_options() { let sql = "CREATE EXTERNAL TABLE city ( diff --git a/src/sql/src/statements/create.rs b/src/sql/src/statements/create.rs index 80de3472fc43..e0c9cdea88ab 100644 --- a/src/sql/src/statements/create.rs +++ b/src/sql/src/statements/create.rs @@ -219,6 +219,13 @@ pub struct CreateExternalTable { pub engine: String, } +#[derive(Debug, PartialEq, Eq, Clone, Visit, VisitMut)] +pub struct CreateTableLike { + /// Table name + pub name: ObjectName, + pub target: ObjectName, +} + #[cfg(test)] mod tests { use crate::dialect::GreptimeDbDialect; diff --git a/src/sql/src/statements/statement.rs b/src/sql/src/statements/statement.rs index 028f437c3daf..b8af789616d1 100644 --- a/src/sql/src/statements/statement.rs +++ b/src/sql/src/statements/statement.rs @@ -19,7 +19,9 @@ use sqlparser_derive::{Visit, VisitMut}; use super::show::ShowVariables; use crate::error::{ConvertToDfStatementSnafu, Error}; use crate::statements::alter::AlterTable; -use crate::statements::create::{CreateDatabase, CreateExternalTable, CreateTable}; +use crate::statements::create::{ + CreateDatabase, CreateExternalTable, CreateTable, CreateTableLike, +}; use crate::statements::delete::Delete; use crate::statements::describe::DescribeTable; use crate::statements::drop::DropTable; @@ -45,6 +47,8 @@ pub enum Statement { CreateTable(CreateTable), // CREATE EXTERNAL TABLE CreateExternalTable(CreateExternalTable), + // CREATE TABLE ... LIKE + CreateTableLike(CreateTableLike), // DROP TABLE DropTable(DropTable), // CREATE DATABASE diff --git a/tests/cases/standalone/common/create/create.result b/tests/cases/standalone/common/create/create.result index 53cb8bbba0f9..641514a526a4 100644 --- a/tests/cases/standalone/common/create/create.result +++ b/tests/cases/standalone/common/create/create.result @@ -143,3 +143,41 @@ DROP TABLE neg_default_value; Affected Rows: 0 +CREATE TABLE test_like_1 (i INTEGER, j TIMESTAMP TIME INDEX); + +Affected Rows: 0 + +CREATE TABLE test_like_2 LIKE test_like_1; + +Affected Rows: 0 + +CREATE TABLE test_like_2 LIKE test_like_1; + +Error: 4000(TableAlreadyExists), Table already exists: `greptime.public.test_like_2` + +DESC TABLE test_like_1; + ++--------+----------------------+-----+------+---------+---------------+ +| Column | Type | Key | Null | Default | Semantic Type | ++--------+----------------------+-----+------+---------+---------------+ +| i | Int32 | | YES | | FIELD | +| j | TimestampMillisecond | PRI | NO | | TIMESTAMP | ++--------+----------------------+-----+------+---------+---------------+ + +DESC TABLE test_like_2; + ++--------+----------------------+-----+------+---------+---------------+ +| Column | Type | Key | Null | Default | Semantic Type | ++--------+----------------------+-----+------+---------+---------------+ +| i | Int32 | | YES | | FIELD | +| j | TimestampMillisecond | PRI | NO | | TIMESTAMP | ++--------+----------------------+-----+------+---------+---------------+ + +DROP TABLE test_like_1; + +Affected Rows: 0 + +DROP TABLE test_like_2; + +Affected Rows: 0 + diff --git a/tests/cases/standalone/common/create/create.sql b/tests/cases/standalone/common/create/create.sql index 47d95c46d73c..a2e3c630d9d7 100644 --- a/tests/cases/standalone/common/create/create.sql +++ b/tests/cases/standalone/common/create/create.sql @@ -57,3 +57,17 @@ CREATE TABLE neg_default_value(i INT DEFAULT -1024, ts TIMESTAMP TIME INDEX); desc TABLE neg_default_value; DROP TABLE neg_default_value; + +CREATE TABLE test_like_1 (i INTEGER, j TIMESTAMP TIME INDEX); + +CREATE TABLE test_like_2 LIKE test_like_1; + +CREATE TABLE test_like_2 LIKE test_like_1; + +DESC TABLE test_like_1; + +DESC TABLE test_like_2; + +DROP TABLE test_like_1; + +DROP TABLE test_like_2; From e67f7740e25accade467572fcaec25c430d247d3 Mon Sep 17 00:00:00 2001 From: kould Date: Sat, 24 Feb 2024 03:04:39 +0800 Subject: [PATCH 2/5] fix: `check_permission` for `Create Table ... Like` --- src/frontend/src/instance.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/frontend/src/instance.rs b/src/frontend/src/instance.rs index 47757104e855..3c31634a47de 100644 --- a/src/frontend/src/instance.rs +++ b/src/frontend/src/instance.rs @@ -460,10 +460,8 @@ pub fn check_permission( // database ops won't be checked Statement::CreateDatabase(_) | Statement::ShowDatabases(_) => {} // show create table and alter are not supported yet - Statement::ShowCreateTable(_) - | Statement::CreateExternalTable(_) - | Statement::CreateTableLike(_) - | Statement::Alter(_) => {} + Statement::ShowCreateTable(_) | Statement::CreateExternalTable(_) | Statement::Alter(_) => { + } // set/show variable now only alter/show variable in session Statement::SetVariables(_) | Statement::ShowVariables(_) => {} @@ -473,6 +471,10 @@ pub fn check_permission( Statement::CreateTable(stmt) => { validate_param(&stmt.name, query_ctx)?; } + Statement::CreateTableLike(stmt) => { + validate_param(&stmt.name, query_ctx)?; + validate_param(&stmt.target, query_ctx)?; + } Statement::DropTable(drop_stmt) => { validate_param(drop_stmt.table_name(), query_ctx)?; } From 749bc595d9eea03f6708133405b58f82f88589b1 Mon Sep 17 00:00:00 2001 From: kould Date: Wed, 28 Feb 2024 19:23:10 +0800 Subject: [PATCH 3/5] style: renaming `name` -> `table_name` & `target` -> `source_name` and make `Create Table ... Like` testcase more complicated --- src/frontend/src/instance.rs | 4 ++-- src/operator/src/expr_factory.rs | 2 +- src/operator/src/statement/ddl.rs | 4 ++-- src/sql/src/parsers/create_parser.rs | 10 +++++----- src/sql/src/statements/create.rs | 5 +++-- tests/cases/standalone/common/create/create.result | 8 +++++--- tests/cases/standalone/common/create/create.sql | 2 +- 7 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/frontend/src/instance.rs b/src/frontend/src/instance.rs index 3c31634a47de..55c45c71fdd0 100644 --- a/src/frontend/src/instance.rs +++ b/src/frontend/src/instance.rs @@ -472,8 +472,8 @@ pub fn check_permission( validate_param(&stmt.name, query_ctx)?; } Statement::CreateTableLike(stmt) => { - validate_param(&stmt.name, query_ctx)?; - validate_param(&stmt.target, query_ctx)?; + validate_param(&stmt.table_name, query_ctx)?; + validate_param(&stmt.source_name, query_ctx)?; } Statement::DropTable(drop_stmt) => { validate_param(drop_stmt.table_name(), query_ctx)?; diff --git a/src/operator/src/expr_factory.rs b/src/operator/src/expr_factory.rs index 0ad8e65828ff..0975f9b17dbd 100644 --- a/src/operator/src/expr_factory.rs +++ b/src/operator/src/expr_factory.rs @@ -181,7 +181,7 @@ pub(crate) async fn create_external_expr( Ok(expr) } -/// Convert `CreateTable` statement to `CreateTableExpr` gRPC request. +/// Convert `CreateTable` statement to [`CreateTableExpr`] gRPC request. pub fn create_to_expr(create: &CreateTable, query_ctx: QueryContextRef) -> Result { let (catalog_name, schema_name, table_name) = table_idents_to_full_name(&create.name, &query_ctx) diff --git a/src/operator/src/statement/ddl.rs b/src/operator/src/statement/ddl.rs index 86cd9a97ded9..e4e60b65b9c0 100644 --- a/src/operator/src/statement/ddl.rs +++ b/src/operator/src/statement/ddl.rs @@ -81,7 +81,7 @@ impl StatementExecutor { stmt: CreateTableLike, ctx: QueryContextRef, ) -> Result { - let (catalog, schema, table) = table_idents_to_full_name(&stmt.target, &ctx) + let (catalog, schema, table) = table_idents_to_full_name(&stmt.source_name, &ctx) .map_err(BoxedError::new) .context(error::ExternalSnafu)?; let table_ref = self @@ -100,7 +100,7 @@ impl StatementExecutor { let mut create_stmt = show_create_table::create_table_stmt(&table_ref.table_info(), quote_style) .context(error::ParseQuerySnafu)?; - create_stmt.name = stmt.name; + create_stmt.name = stmt.table_name; create_stmt.if_not_exists = false; let partitions = create_partitions_stmt(partitions)?.and_then(|mut partitions| { diff --git a/src/sql/src/parsers/create_parser.rs b/src/sql/src/parsers/create_parser.rs index 63759e0419ba..2f1057222ee4 100644 --- a/src/sql/src/parsers/create_parser.rs +++ b/src/sql/src/parsers/create_parser.rs @@ -131,11 +131,11 @@ impl<'a> ParserContext<'a> { let table_name = self.parse_table_name()?; if self.parser.parse_keyword(Keyword::LIKE) { - let target_table_name = self.parse_table_name()?; + let source_name = self.parse_table_name()?; return Ok(Statement::CreateTableLike(CreateTableLike { - name: table_name, - target: target_table_name, + table_name, + source_name, })); } @@ -754,8 +754,8 @@ mod tests { assert_eq!(1, stmts.len()); match &stmts[0] { Statement::CreateTableLike(c) => { - assert_eq!(c.name.to_string(), "t1"); - assert_eq!(c.target.to_string(), "t2"); + assert_eq!(c.table_name.to_string(), "t1"); + assert_eq!(c.source_name.to_string(), "t2"); } _ => unreachable!(), } diff --git a/src/sql/src/statements/create.rs b/src/sql/src/statements/create.rs index e0c9cdea88ab..5cc6b4ccca13 100644 --- a/src/sql/src/statements/create.rs +++ b/src/sql/src/statements/create.rs @@ -222,8 +222,9 @@ pub struct CreateExternalTable { #[derive(Debug, PartialEq, Eq, Clone, Visit, VisitMut)] pub struct CreateTableLike { /// Table name - pub name: ObjectName, - pub target: ObjectName, + pub table_name: ObjectName, + /// The table that is designated to be imitated by `Like` + pub source_name: ObjectName, } #[cfg(test)] diff --git a/tests/cases/standalone/common/create/create.result b/tests/cases/standalone/common/create/create.result index 641514a526a4..664c94fedebc 100644 --- a/tests/cases/standalone/common/create/create.result +++ b/tests/cases/standalone/common/create/create.result @@ -143,7 +143,7 @@ DROP TABLE neg_default_value; Affected Rows: 0 -CREATE TABLE test_like_1 (i INTEGER, j TIMESTAMP TIME INDEX); +CREATE TABLE test_like_1 (PK STRING PRIMARY KEY, i INTEGER DEFAULT 7, j TIMESTAMP TIME INDEX); Affected Rows: 0 @@ -160,7 +160,8 @@ DESC TABLE test_like_1; +--------+----------------------+-----+------+---------+---------------+ | Column | Type | Key | Null | Default | Semantic Type | +--------+----------------------+-----+------+---------+---------------+ -| i | Int32 | | YES | | FIELD | +| pk | String | PRI | YES | | TAG | +| i | Int32 | | YES | 7 | FIELD | | j | TimestampMillisecond | PRI | NO | | TIMESTAMP | +--------+----------------------+-----+------+---------+---------------+ @@ -169,7 +170,8 @@ DESC TABLE test_like_2; +--------+----------------------+-----+------+---------+---------------+ | Column | Type | Key | Null | Default | Semantic Type | +--------+----------------------+-----+------+---------+---------------+ -| i | Int32 | | YES | | FIELD | +| pk | String | PRI | YES | | TAG | +| i | Int32 | | YES | 7 | FIELD | | j | TimestampMillisecond | PRI | NO | | TIMESTAMP | +--------+----------------------+-----+------+---------+---------------+ diff --git a/tests/cases/standalone/common/create/create.sql b/tests/cases/standalone/common/create/create.sql index a2e3c630d9d7..f27b2bc6e812 100644 --- a/tests/cases/standalone/common/create/create.sql +++ b/tests/cases/standalone/common/create/create.sql @@ -58,7 +58,7 @@ desc TABLE neg_default_value; DROP TABLE neg_default_value; -CREATE TABLE test_like_1 (i INTEGER, j TIMESTAMP TIME INDEX); +CREATE TABLE test_like_1 (PK STRING PRIMARY KEY, i INTEGER DEFAULT 7, j TIMESTAMP TIME INDEX); CREATE TABLE test_like_2 LIKE test_like_1; From 86a1a4c0c2bb2b477b663442ee97440e088dbfa3 Mon Sep 17 00:00:00 2001 From: kould Date: Wed, 28 Feb 2024 20:36:55 +0800 Subject: [PATCH 4/5] rebase --- src/operator/src/statement/ddl.rs | 4 ++-- src/sql/src/parser.rs | 7 +++++-- src/sql/src/parsers/create_parser.rs | 20 ++++---------------- 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/src/operator/src/statement/ddl.rs b/src/operator/src/statement/ddl.rs index e4e60b65b9c0..9f5a71ec7fe6 100644 --- a/src/operator/src/statement/ddl.rs +++ b/src/operator/src/statement/ddl.rs @@ -38,9 +38,10 @@ use partition::partition::{PartitionBound, PartitionDef}; use query::sql::show_create_table; use regex::Regex; use session::context::QueryContextRef; +use session::table_name::table_idents_to_full_name; use snafu::{ensure, IntoError, OptionExt, ResultExt}; use sql::statements::alter::AlterTable; -use sql::statements::create::{CreateExternalTable, CreateTable, Partitions}; +use sql::statements::create::{CreateExternalTable, CreateTable, CreateTableLike, Partitions}; use table::dist_table::DistTable; use table::metadata::{self, RawTableInfo, RawTableMeta, TableId, TableInfo, TableType}; use table::requests::{AlterKind, AlterTableRequest, TableOptions}; @@ -57,7 +58,6 @@ use crate::error::{ }; use crate::expr_factory; use crate::statement::show::create_partitions_stmt; -use crate::table::table_idents_to_full_name; lazy_static! { static ref NAME_PATTERN_REG: Regex = Regex::new(&format!("^{NAME_PATTERN}$")).unwrap(); diff --git a/src/sql/src/parser.rs b/src/sql/src/parser.rs index 52aaf0213bd4..46e47401f39d 100644 --- a/src/sql/src/parser.rs +++ b/src/sql/src/parser.rs @@ -80,14 +80,17 @@ impl<'a> ParserContext<'a> { .try_with_sql(sql) .context(SyntaxSnafu)?; + Self::_parse_table_name(&mut parser, sql) + } + + pub(crate) fn _parse_table_name(parser: &mut Parser, sql: &'a str) -> Result { let raw_table_name = parser.parse_object_name().context(error::UnexpectedSnafu { sql, expected: "a table name", actual: parser.peek_token().to_string(), })?; - let table_name = Self::canonicalize_object_name(raw_table_name); - Ok(table_name) + Ok(Self::canonicalize_object_name(raw_table_name)) } pub fn parse_function(sql: &'a str, dialect: &dyn Dialect) -> Result { diff --git a/src/sql/src/parsers/create_parser.rs b/src/sql/src/parsers/create_parser.rs index 2f1057222ee4..63e9d7641c8b 100644 --- a/src/sql/src/parsers/create_parser.rs +++ b/src/sql/src/parsers/create_parser.rs @@ -32,7 +32,7 @@ use crate::error::{ }; use crate::parser::ParserContext; use crate::statements::create::{ - CreateDatabase, CreateExternalTable, CreateTable, Partitions, TIME_INDEX, + CreateDatabase, CreateExternalTable, CreateTable, CreateTableLike, Partitions, TIME_INDEX, }; use crate::statements::get_data_type_by_alias_name; use crate::statements::statement::Statement; @@ -66,7 +66,7 @@ impl<'a> ParserContext<'a> { let if_not_exists = self.parser .parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]); - let table_name = self.parse_table_name()?; + let table_name = ParserContext::_parse_table_name(&mut self.parser, self.sql)?; let (columns, constraints) = self.parse_columns()?; let engine = self.parse_table_engine(common_catalog::consts::FILE_ENGINE)?; let options = self @@ -128,10 +128,10 @@ impl<'a> ParserContext<'a> { self.parser .parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]); - let table_name = self.parse_table_name()?; + let table_name = ParserContext::_parse_table_name(&mut self.parser, self.sql)?; if self.parser.parse_keyword(Keyword::LIKE) { - let source_name = self.parse_table_name()?; + let source_name = ParserContext::_parse_table_name(&mut self.parser, self.sql)?; return Ok(Statement::CreateTableLike(CreateTableLike { table_name, @@ -173,18 +173,6 @@ impl<'a> ParserContext<'a> { Ok(Statement::CreateTable(create_table)) } - fn parse_table_name(&mut self) -> Result { - let raw_table_name = self - .parser - .parse_object_name() - .context(error::UnexpectedSnafu { - sql: self.sql, - expected: "a table name", - actual: self.peek_token_as_string(), - })?; - Ok(Self::canonicalize_object_name(raw_table_name)) - } - /// "PARTITION BY ..." syntax: // TODO(ruihang): docs fn parse_partitions(&mut self) -> Result> { From 0d5948e399529c61e6eeb1c29e3a63701c5e60c6 Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 2 Mar 2024 14:18:04 +0800 Subject: [PATCH 5/5] avoid _ fn Signed-off-by: tison --- src/operator/src/statement/ddl.rs | 13 ++++++------- src/query/src/sql.rs | 5 +++-- src/sql/src/parser.rs | 21 +++++++++++---------- src/sql/src/parsers/create_parser.rs | 6 +++--- 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/operator/src/statement/ddl.rs b/src/operator/src/statement/ddl.rs index 9f5a71ec7fe6..88cb04bc7eba 100644 --- a/src/operator/src/statement/ddl.rs +++ b/src/operator/src/statement/ddl.rs @@ -35,7 +35,7 @@ use datatypes::prelude::ConcreteDataType; use datatypes::schema::RawSchema; use lazy_static::lazy_static; use partition::partition::{PartitionBound, PartitionDef}; -use query::sql::show_create_table; +use query::sql::create_table_stmt; use regex::Regex; use session::context::QueryContextRef; use session::table_name::table_idents_to_full_name; @@ -88,8 +88,8 @@ impl StatementExecutor { .catalog_manager .table(&catalog, &schema, &table) .await - .context(error::CatalogSnafu)? - .context(error::TableNotFoundSnafu { table_name: &table })?; + .context(CatalogSnafu)? + .context(TableNotFoundSnafu { table_name: &table })?; let partitions = self .partition_manager .find_table_partitions(table_ref.table_info().table_id()) @@ -97,9 +97,8 @@ impl StatementExecutor { .context(error::FindTablePartitionRuleSnafu { table_name: table })?; let quote_style = ctx.quote_style(); - let mut create_stmt = - show_create_table::create_table_stmt(&table_ref.table_info(), quote_style) - .context(error::ParseQuerySnafu)?; + let mut create_stmt = create_table_stmt(&table_ref.table_info(), quote_style) + .context(error::ParseQuerySnafu)?; create_stmt.name = stmt.table_name; create_stmt.if_not_exists = false; @@ -205,7 +204,7 @@ impl StatementExecutor { table_info.ident.table_id = table_id; - let table_info = Arc::new(table_info.try_into().context(error::CreateTableInfoSnafu)?); + let table_info = Arc::new(table_info.try_into().context(CreateTableInfoSnafu)?); create_table.table_id = Some(api::v1::TableId { id: table_id }); let table = DistTable::table(table_info); diff --git a/src/query/src/sql.rs b/src/query/src/sql.rs index e170f1208473..34e3f17fdd35 100644 --- a/src/query/src/sql.rs +++ b/src/query/src/sql.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -pub mod show_create_table; +mod show_create_table; use std::collections::HashMap; use std::sync::Arc; @@ -37,6 +37,7 @@ use object_store::ObjectStore; use once_cell::sync::Lazy; use regex::Regex; use session::context::QueryContextRef; +pub use show_create_table::create_table_stmt; use snafu::{ensure, OptionExt, ResultExt}; use sql::statements::create::Partitions; use sql::statements::show::{ShowDatabases, ShowKind, ShowTables, ShowVariables}; @@ -260,7 +261,7 @@ pub fn show_create_table( let quote_style = query_ctx.quote_style(); - let mut stmt = show_create_table::create_table_stmt(&table_info, quote_style)?; + let mut stmt = create_table_stmt(&table_info, quote_style)?; stmt.partitions = partitions.map(|mut p| { p.set_quote(quote_style); p diff --git a/src/sql/src/parser.rs b/src/sql/src/parser.rs index 46e47401f39d..9010106d850a 100644 --- a/src/sql/src/parser.rs +++ b/src/sql/src/parser.rs @@ -75,21 +75,22 @@ impl<'a> ParserContext<'a> { } pub fn parse_table_name(sql: &'a str, dialect: &dyn Dialect) -> Result { - let mut parser = Parser::new(dialect) + let parser = Parser::new(dialect) .with_options(ParserOptions::new().with_trailing_commas(true)) .try_with_sql(sql) .context(SyntaxSnafu)?; - - Self::_parse_table_name(&mut parser, sql) + ParserContext { parser, sql }.intern_parse_table_name() } - pub(crate) fn _parse_table_name(parser: &mut Parser, sql: &'a str) -> Result { - let raw_table_name = parser.parse_object_name().context(error::UnexpectedSnafu { - sql, - expected: "a table name", - actual: parser.peek_token().to_string(), - })?; - + pub(crate) fn intern_parse_table_name(&mut self) -> Result { + let raw_table_name = self + .parser + .parse_object_name() + .context(error::UnexpectedSnafu { + sql: self.sql, + expected: "a table name", + actual: self.parser.peek_token().to_string(), + })?; Ok(Self::canonicalize_object_name(raw_table_name)) } diff --git a/src/sql/src/parsers/create_parser.rs b/src/sql/src/parsers/create_parser.rs index 63e9d7641c8b..00a797d44b23 100644 --- a/src/sql/src/parsers/create_parser.rs +++ b/src/sql/src/parsers/create_parser.rs @@ -66,7 +66,7 @@ impl<'a> ParserContext<'a> { let if_not_exists = self.parser .parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]); - let table_name = ParserContext::_parse_table_name(&mut self.parser, self.sql)?; + let table_name = self.intern_parse_table_name()?; let (columns, constraints) = self.parse_columns()?; let engine = self.parse_table_engine(common_catalog::consts::FILE_ENGINE)?; let options = self @@ -128,10 +128,10 @@ impl<'a> ParserContext<'a> { self.parser .parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]); - let table_name = ParserContext::_parse_table_name(&mut self.parser, self.sql)?; + let table_name = self.intern_parse_table_name()?; if self.parser.parse_keyword(Keyword::LIKE) { - let source_name = ParserContext::_parse_table_name(&mut self.parser, self.sql)?; + let source_name = self.intern_parse_table_name()?; return Ok(Statement::CreateTableLike(CreateTableLike { table_name,