chore: check catalog deregister result (#1810)

* chore: check deregister result and return error on failure

* refactor: SystemCatalog::deregister_table returns Result<()>
This commit is contained in:
Lei, HUANG
2023-06-21 16:09:11 +08:00
committed by GitHub
parent a314993ab4
commit d1b5ce0d35
5 changed files with 31 additions and 11 deletions

View File

@@ -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
}
}

View File

@@ -662,7 +662,7 @@ impl CatalogManager for RemoteCatalogManager {
.await;
}
Ok(result.is_none())
Ok(true)
}
async fn register_schema(&self, request: RegisterSchemaRequest) -> Result<bool> {

View File

@@ -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<bool> {
) -> 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(),
})

View File

@@ -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;

View File

@@ -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<T> = std::result::Result<T, Error>;
@@ -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 { .. } => {