diff --git a/src/catalog/src/local/manager.rs b/src/catalog/src/local/manager.rs index 1d66e2b961..3dd9d777fd 100644 --- a/src/catalog/src/local/manager.rs +++ b/src/catalog/src/local/manager.rs @@ -467,10 +467,7 @@ impl CatalogManager for LocalCatalogManager { .ident .table_id; - if !self.system.deregister_table(&request, table_id).await? { - return Ok(false); - } - + self.system.deregister_table(&request, table_id).await?; self.catalogs.deregister_table(request).await } } diff --git a/src/catalog/src/remote/manager.rs b/src/catalog/src/remote/manager.rs index eea2ddf67a..36545df835 100644 --- a/src/catalog/src/remote/manager.rs +++ b/src/catalog/src/remote/manager.rs @@ -662,7 +662,7 @@ impl CatalogManager for RemoteCatalogManager { .await; } - Ok(result.is_none()) + Ok(true) } async fn register_schema(&self, request: RegisterSchemaRequest) -> Result { diff --git a/src/catalog/src/tables.rs b/src/catalog/src/tables.rs index baf8e24fa2..175e0799a7 100644 --- a/src/catalog/src/tables.rs +++ b/src/catalog/src/tables.rs @@ -19,6 +19,7 @@ use std::sync::Arc; use async_trait::async_trait; use common_catalog::consts::{INFORMATION_SCHEMA_NAME, SYSTEM_CATALOG_TABLE_NAME}; +use common_telemetry::logging; use snafu::ResultExt; use table::metadata::TableId; use table::{Table, TableRef}; @@ -91,12 +92,21 @@ impl SystemCatalog { &self, request: &DeregisterTableRequest, table_id: TableId, - ) -> CatalogResult { + ) -> CatalogResult<()> { self.information_schema .system .delete(build_table_deletion_request(request, table_id)) .await - .map(|x| x == 1) + .map(|x| { + if x != 1 { + let table = common_catalog::format_full_table_name( + &request.catalog, + &request.schema, + &request.table_name + ); + logging::warn!("Failed to delete table record from information_schema, unexpected returned result: {x}, table: {table}"); + } + }) .with_context(|_| error::DeregisterTableSnafu { request: request.clone(), }) diff --git a/src/table-procedure/src/drop.rs b/src/table-procedure/src/drop.rs index 9c436f6a9c..9a8c5314b4 100644 --- a/src/table-procedure/src/drop.rs +++ b/src/table-procedure/src/drop.rs @@ -28,7 +28,8 @@ use table::engine::{EngineContext, TableEngineProcedureRef, TableReference}; use table::requests::DropTableRequest; use crate::error::{ - AccessCatalogSnafu, DeserializeProcedureSnafu, SerializeProcedureSnafu, TableNotFoundSnafu, + AccessCatalogSnafu, DeregisterTableSnafu, DeserializeProcedureSnafu, SerializeProcedureSnafu, + TableNotFoundSnafu, }; /// Procedure to drop a table. @@ -158,10 +159,17 @@ impl DropTableProcedure { schema: self.data.request.schema_name.clone(), table_name: self.data.request.table_name.clone(), }; - self.catalog_manager + if !self + .catalog_manager .deregister_table(deregister_table_req) .await - .context(AccessCatalogSnafu)?; + .context(AccessCatalogSnafu)? + { + return DeregisterTableSnafu { + name: request.table_ref().to_string(), + } + .fail()?; + } } self.data.state = DropTableState::EngineDropTable; diff --git a/src/table-procedure/src/error.rs b/src/table-procedure/src/error.rs index 25cca7c910..9ab6ec0c96 100644 --- a/src/table-procedure/src/error.rs +++ b/src/table-procedure/src/error.rs @@ -55,6 +55,9 @@ pub enum Error { #[snafu(display("Table already exists: {}", name))] TableExists { name: String }, + + #[snafu(display("Failed to deregister table: {}", name))] + DeregisterTable { name: String }, } pub type Result = std::result::Result; @@ -64,7 +67,9 @@ impl ErrorExt for Error { use Error::*; match self { - SerializeProcedure { .. } | DeserializeProcedure { .. } => StatusCode::Internal, + DeregisterTable { .. } | SerializeProcedure { .. } | DeserializeProcedure { .. } => { + StatusCode::Internal + } InvalidRawSchema { source, .. } => source.status_code(), AccessCatalog { source, .. } => source.status_code(), CatalogNotFound { .. } | SchemaNotFound { .. } | TableExists { .. } => {