From ccd6de8d6bf9355d3eec329ef2e830adeefd588b Mon Sep 17 00:00:00 2001 From: LFC <990479+MichaelScofield@users.noreply.github.com> Date: Wed, 27 Sep 2023 19:50:07 +0800 Subject: [PATCH] fix: allow `.`(dot) literal in table name (#2483) * fix: allow `.`(dot) literal in table name * fix: resolve PR comments --- src/common/catalog/src/lib.rs | 14 -------------- src/common/meta/src/key.rs | 2 +- src/common/meta/src/key/table_name.rs | 2 ++ src/meta-srv/src/service/admin/meta.rs | 19 +++++++++---------- src/meta-srv/src/service/admin/route.rs | 21 ++++++++++----------- 5 files changed, 22 insertions(+), 36 deletions(-) diff --git a/src/common/catalog/src/lib.rs b/src/common/catalog/src/lib.rs index 79e6de5a0dd0..1527b7f4a063 100644 --- a/src/common/catalog/src/lib.rs +++ b/src/common/catalog/src/lib.rs @@ -13,9 +13,6 @@ // limitations under the License. use consts::DEFAULT_CATALOG_NAME; -use snafu::ensure; - -use crate::error::Result; pub mod consts; pub mod error; @@ -26,17 +23,6 @@ pub fn format_full_table_name(catalog: &str, schema: &str, table: &str) -> Strin format!("{catalog}.{schema}.{table}") } -pub fn parse_full_table_name(table_name: &str) -> Result<(&str, &str, &str)> { - let result = table_name.split('.').collect::>(); - - ensure!( - result.len() == 3, - error::InvalidFullTableNameSnafu { table_name } - ); - - Ok((result[0], result[1], result[2])) -} - /// Build db name from catalog and schema string pub fn build_db_string(catalog: &str, schema: &str) -> String { if catalog == DEFAULT_CATALOG_NAME { diff --git a/src/common/meta/src/key.rs b/src/common/meta/src/key.rs index b616725f366f..e0aa5c1ec9dc 100644 --- a/src/common/meta/src/key.rs +++ b/src/common/meta/src/key.rs @@ -80,7 +80,7 @@ use crate::DatanodeId; pub const REMOVED_PREFIX: &str = "__removed"; -const NAME_PATTERN: &str = "[a-zA-Z_:-][a-zA-Z0-9_:-]*"; +const NAME_PATTERN: &str = r"[a-zA-Z_:-][a-zA-Z0-9_:\-\.]*"; const DATANODE_TABLE_KEY_PREFIX: &str = "__dn_table"; const TABLE_INFO_KEY_PREFIX: &str = "__table_info"; diff --git a/src/common/meta/src/key/table_name.rs b/src/common/meta/src/key/table_name.rs index a4b53de00b04..e87234c603d5 100644 --- a/src/common/meta/src/key/table_name.rs +++ b/src/common/meta/src/key/table_name.rs @@ -268,6 +268,8 @@ mod tests { test_ok("my_table"); test_ok("cpu:metrics"); test_ok(":cpu:metrics"); + test_ok("sys.cpu.system"); + test_ok("foo-bar"); } #[test] diff --git a/src/meta-srv/src/service/admin/meta.rs b/src/meta-srv/src/service/admin/meta.rs index 8b9028117eaa..d199edd388ec 100644 --- a/src/meta-srv/src/service/admin/meta.rs +++ b/src/meta-srv/src/service/admin/meta.rs @@ -14,7 +14,6 @@ use std::collections::HashMap; -use common_catalog::parse_full_table_name; use common_error::ext::BoxedError; use common_meta::key::table_name::TableNameKey; use common_meta::key::TableMetadataManagerRef; @@ -129,15 +128,15 @@ impl HttpHandler for TableHandler { _: &str, params: &HashMap, ) -> Result> { - let table_name = - params - .get("full_table_name") - .context(error::MissingRequiredParameterSnafu { - param: "full_table_name", - })?; - - let (catalog, schema, table) = - parse_full_table_name(table_name).context(error::InvalidFullTableNameSnafu)?; + let catalog = params + .get("catalog") + .context(error::MissingRequiredParameterSnafu { param: "catalog" })?; + let schema = params + .get("schema") + .context(error::MissingRequiredParameterSnafu { param: "schema" })?; + let table = params + .get("table") + .context(error::MissingRequiredParameterSnafu { param: "table" })?; let key = TableNameKey::new(catalog, schema, table); diff --git a/src/meta-srv/src/service/admin/route.rs b/src/meta-srv/src/service/admin/route.rs index 80da536d583a..ad8c0fdaf55c 100644 --- a/src/meta-srv/src/service/admin/route.rs +++ b/src/meta-srv/src/service/admin/route.rs @@ -14,7 +14,6 @@ use std::collections::HashMap; -use common_catalog::parse_full_table_name; use common_meta::key::table_name::TableNameKey; use common_meta::key::TableMetadataManagerRef; use snafu::{OptionExt, ResultExt}; @@ -35,15 +34,15 @@ impl HttpHandler for RouteHandler { _path: &str, params: &HashMap, ) -> Result> { - let table_name = - params - .get("full_table_name") - .context(error::MissingRequiredParameterSnafu { - param: "full_table_name", - })?; - - let (catalog, schema, table) = - parse_full_table_name(table_name).context(error::InvalidFullTableNameSnafu)?; + let catalog = params + .get("catalog") + .context(error::MissingRequiredParameterSnafu { param: "catalog" })?; + let schema = params + .get("schema") + .context(error::MissingRequiredParameterSnafu { param: "schema" })?; + let table = params + .get("table") + .context(error::MissingRequiredParameterSnafu { param: "table" })?; let key = TableNameKey::new(catalog, schema, table); @@ -54,7 +53,7 @@ impl HttpHandler for RouteHandler { .await .context(error::TableMetadataManagerSnafu)? .map(|x| x.table_id()) - .context(TableNotFoundSnafu { name: table_name })?; + .context(TableNotFoundSnafu { name: table })?; let table_route_value = self .table_metadata_manager