diff --git a/src/frontend/src/instance.rs b/src/frontend/src/instance.rs index 81ebe805cb..4a21874df3 100644 --- a/src/frontend/src/instance.rs +++ b/src/frontend/src/instance.rs @@ -36,7 +36,6 @@ use common_query::Output; use common_recordbatch::RecordBatches; use common_telemetry::logging::{debug, info}; use datafusion::sql::sqlparser::ast::ObjectName; -use datafusion_common::TableReference; use datanode::instance::sql::table_idents_to_full_name; use datanode::instance::InstanceRef as DnInstanceRef; use datatypes::schema::Schema; @@ -45,7 +44,7 @@ use meta_client::client::{MetaClient, MetaClientBuilder}; use meta_client::MetaClientOpts; use partition::manager::PartitionRuleManager; use partition::route::TableRoutes; -use query::query_engine::options::QueryOptions; +use query::query_engine::options::{validate_catalog_and_schema, QueryOptions}; use servers::error as server_error; use servers::interceptor::{SqlQueryInterceptor, SqlQueryInterceptorRef}; use servers::promql::{PromqlHandler, PromqlHandlerRef}; @@ -558,64 +557,34 @@ pub fn check_permission( Statement::ShowCreateTable(_) | Statement::Alter(_) => {} Statement::Insert(insert) => { - let (catalog, schema, _) = - table_idents_to_full_name(insert.table_name(), query_ctx.clone()) - .map_err(BoxedError::new) - .context(ExternalSnafu)?; - - validate_param(&catalog, &schema, query_ctx)?; + validate_param(insert.table_name(), query_ctx)?; } Statement::CreateTable(stmt) => { - let tab_ref = obj_name_to_tab_ref(&stmt.name)?; - validate_tab_ref(tab_ref, query_ctx)?; + validate_param(&stmt.name, query_ctx)?; } Statement::DropTable(drop_stmt) => { - let tab_ref = obj_name_to_tab_ref(drop_stmt.table_name())?; - validate_tab_ref(tab_ref, query_ctx)?; + validate_param(drop_stmt.table_name(), query_ctx)?; } Statement::ShowTables(stmt) => { if let Some(database) = &stmt.database { - validate_param(&query_ctx.current_catalog(), database, query_ctx)?; + validate_catalog_and_schema(&query_ctx.current_catalog(), database, query_ctx) + .map_err(BoxedError::new) + .context(SqlExecInterceptedSnafu)?; } } Statement::DescribeTable(stmt) => { - let tab_ref = obj_name_to_tab_ref(stmt.name())?; - validate_tab_ref(tab_ref, query_ctx)?; + validate_param(stmt.name(), query_ctx)?; } } Ok(()) } -fn obj_name_to_tab_ref(obj: &ObjectName) -> Result { - match &obj.0[..] { - [table] => Ok(TableReference::Bare { - table: &table.value, - }), - [schema, table] => Ok(TableReference::Partial { - schema: &schema.value, - table: &table.value, - }), - [catalog, schema, table] => Ok(TableReference::Full { - catalog: &catalog.value, - schema: &schema.value, - table: &table.value, - }), - _ => error::InvalidSqlSnafu { - err_msg: format!( - "expect table name to be .., .
or
, actual: {obj}", - ), - }.fail(), - } -} - -fn validate_tab_ref(tab_ref: TableReference, query_ctx: &QueryContextRef) -> Result<()> { - query::query_engine::options::validate_table_references(tab_ref, query_ctx) +fn validate_param(name: &ObjectName, query_ctx: &QueryContextRef) -> Result<()> { + let (catalog, schema, _) = table_idents_to_full_name(name, query_ctx.clone()) .map_err(BoxedError::new) - .context(SqlExecInterceptedSnafu) -} + .context(ExternalSnafu)?; -fn validate_param(catalog: &str, schema: &str, query_ctx: &QueryContextRef) -> Result<()> { - query::query_engine::options::validate_catalog_and_schema(catalog, schema, query_ctx) + validate_catalog_and_schema(&catalog, &schema, query_ctx) .map_err(BoxedError::new) .context(SqlExecInterceptedSnafu) } diff --git a/src/query/src/query_engine/options.rs b/src/query/src/query_engine/options.rs index 4bf10401d7..00f584ad0d 100644 --- a/src/query/src/query_engine/options.rs +++ b/src/query/src/query_engine/options.rs @@ -23,6 +23,7 @@ pub struct QueryOptions { pub disallow_cross_schema_query: bool, } +// TODO(shuiyisong): remove one method after #559 is done pub fn validate_catalog_and_schema( catalog: &str, schema: &str,