Skip to content

Commit

Permalink
fix: allow .(dot) literal in table name (#2483)
Browse files Browse the repository at this point in the history
* fix: allow `.`(dot) literal in table name

* fix: resolve PR comments
  • Loading branch information
MichaelScofield authored Sep 27, 2023
1 parent ee8d472 commit ccd6de8
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 36 deletions.
14 changes: 0 additions & 14 deletions src/common/catalog/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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::<Vec<_>>();

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 {
Expand Down
2 changes: 1 addition & 1 deletion src/common/meta/src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
2 changes: 2 additions & 0 deletions src/common/meta/src/key/table_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
19 changes: 9 additions & 10 deletions src/meta-srv/src/service/admin/meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -129,15 +128,15 @@ impl HttpHandler for TableHandler {
_: &str,
params: &HashMap<String, String>,
) -> Result<http::Response<String>> {
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);

Expand Down
21 changes: 10 additions & 11 deletions src/meta-srv/src/service/admin/route.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -35,15 +34,15 @@ impl HttpHandler for RouteHandler {
_path: &str,
params: &HashMap<String, String>,
) -> Result<http::Response<String>> {
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);

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

0 comments on commit ccd6de8

Please sign in to comment.