diff --git a/src/binaries/metabench/main.rs b/src/binaries/metabench/main.rs index 7309bbdc64fd..a21e2823588a 100644 --- a/src/binaries/metabench/main.rs +++ b/src/binaries/metabench/main.rs @@ -203,6 +203,10 @@ async fn benchmark_table(client: &Arc, prefix: u64, client_num: u6 .await; print_res(i, "create_db", &res); + let db_id = match res { + Ok(res) => res.db_id, + Err(_) => 0, + }; let res = client .create_table(CreateTableReq { @@ -236,6 +240,8 @@ async fn benchmark_table(client: &Arc, prefix: u64, client_num: u6 .drop_table_by_id(DropTableByIdReq { if_exists: false, tenant: tenant(), + db_id, + table_name: table_name(), tb_id: t.ident.table_id, }) .await; diff --git a/src/meta/api/src/schema_api_impl.rs b/src/meta/api/src/schema_api_impl.rs index d798ffa037f1..5b2a6174dbac 100644 --- a/src/meta/api/src/schema_api_impl.rs +++ b/src/meta/api/src/schema_api_impl.rs @@ -188,6 +188,7 @@ use log::as_debug; use log::as_display; use log::debug; use log::error; +use log::warn; use minitrace::func_name; use ConditionResult::Eq; @@ -2342,12 +2343,20 @@ impl + ?Sized> SchemaApi for KV { let (_, table_name_opt): (_, Option) = get_pb_value(self, &table_id_to_name).await?; - let dbid_tbname = table_name_opt.ok_or_else(|| { - KVAppError::AppError(AppError::UnknownTableId(UnknownTableId::new( - table_id, - "drop_table_by_id failed to find db_id", - ))) - })?; + let dbid_tbname = if let Some(db_id_table_name) = table_name_opt { + db_id_table_name + } else { + let dbid_tbname = DBIdTableName { + db_id: req.db_id, + table_name: req.table_name.clone(), + }; + warn!( + "drop_table_by_id cannot find {:?}, use {:?} instead", + table_id_to_name, dbid_tbname + ); + + dbid_tbname + }; let db_id = dbid_tbname.db_id; let tbname = dbid_tbname.table_name.clone(); diff --git a/src/meta/api/src/schema_api_test_suite.rs b/src/meta/api/src/schema_api_test_suite.rs index 451be73e18b3..ec0714c5e7b0 100644 --- a/src/meta/api/src/schema_api_test_suite.rs +++ b/src/meta/api/src/schema_api_test_suite.rs @@ -326,6 +326,9 @@ impl SchemaApiTestSuite { .await?; suite.catalog_create_get_list_drop(&b.build().await).await?; suite.table_least_visible_time(&b.build().await).await?; + suite + .drop_table_without_tableid_to_name(&b.build().await) + .await?; Ok(()) } @@ -1402,6 +1405,78 @@ impl SchemaApiTestSuite { Ok(()) } + #[minitrace::trace] + async fn drop_table_without_tableid_to_name< + MT: SchemaApi + kvapi::AsKVApi, + >( + &self, + mt: &MT, + ) -> anyhow::Result<()> { + let tenant = "tenant1"; + let db = "db"; + let table_name = "tbl"; + + let create_db_req = CreateDatabaseReq { + if_not_exists: false, + name_ident: DatabaseNameIdent { + tenant: tenant.to_string(), + db_name: db.to_string(), + }, + meta: DatabaseMeta { + engine: "".to_string(), + ..DatabaseMeta::default() + }, + }; + + let res = mt.create_database(create_db_req.clone()).await?; + let db_id = res.db_id; + + let schema = || { + Arc::new(TableSchema::new(vec![TableField::new( + "number", + TableDataType::Number(NumberDataType::UInt64), + )])) + }; + + let table_meta = |created_on| TableMeta { + schema: schema(), + engine: "JSON".to_string(), + options: BTreeMap::new(), + updated_on: created_on, + created_on, + ..TableMeta::default() + }; + let created_on = Utc::now(); + + let req = CreateTableReq { + if_not_exists: false, + name_ident: TableNameIdent { + tenant: tenant.to_string(), + db_name: db.to_string(), + table_name: table_name.to_string(), + }, + + table_meta: table_meta(created_on), + }; + let resp = mt.create_table(req.clone()).await?; + let table_id = resp.table_id; + + let table_id_to_name = TableIdToName { table_id }; + // delete TableIdToName before drop table + delete_test_data(mt.as_kv_api(), &table_id_to_name).await?; + + mt.drop_table_by_id(DropTableByIdReq { + if_exists: false, + tenant: tenant.to_string(), + db_id, + table_name: table_name.to_string(), + tb_id: table_id, + }) + .await?; + + Ok(()) + } + #[minitrace::trace] async fn table_least_visible_time(&self, mt: &MT) -> anyhow::Result<()> { let tenant = "tenant1"; @@ -1568,7 +1643,7 @@ impl SchemaApiTestSuite { } info!("--- prepare db"); - { + let db_id = { let plan = CreateDatabaseReq { if_not_exists: false, name_ident: DatabaseNameIdent { @@ -1585,7 +1660,8 @@ impl SchemaApiTestSuite { info!("create database res: {:?}", res); assert_eq!(1, res.db_id, "first database id is 1"); - } + res.db_id + }; // check table count info!("--- check table count of tenant1"); @@ -1741,6 +1817,8 @@ impl SchemaApiTestSuite { let plan = DropTableByIdReq { if_exists: false, tenant: tenant.to_string(), + db_id, + table_name: tbl_name.to_string(), tb_id, }; mt.drop_table_by_id(plan.clone()).await?; @@ -1768,6 +1846,8 @@ impl SchemaApiTestSuite { let plan = DropTableByIdReq { if_exists: false, tenant: tenant.to_string(), + db_id, + table_name: tbl_name.to_string(), tb_id, }; let res = mt.drop_table_by_id(plan).await; @@ -1785,6 +1865,8 @@ impl SchemaApiTestSuite { let plan = DropTableByIdReq { if_exists: true, tenant: tenant.to_string(), + db_id, + table_name: tbl_name.to_string(), tb_id, }; mt.drop_table_by_id(plan.clone()).await?; @@ -3651,7 +3733,9 @@ impl SchemaApiTestSuite { mt.drop_table_by_id(DropTableByIdReq { if_exists: false, - tenant: tenant.to_string(), + tenant: req.name_ident.tenant.clone(), + db_id, + table_name: req.name_ident.table_name.clone(), tb_id: resp.table_id, }) .await?; @@ -3673,7 +3757,9 @@ impl SchemaApiTestSuite { let resp = mt.create_table(req.clone()).await?; mt.drop_table_by_id(DropTableByIdReq { if_exists: false, - tenant: tenant.to_string(), + tenant: req.name_ident.tenant.clone(), + db_id, + table_name: req.name_ident.table_name.clone(), tb_id: resp.table_id, }) .await?; @@ -3749,7 +3835,9 @@ impl SchemaApiTestSuite { drop_ids_2.push(DroppedId::Table(db_id, resp.table_id, "tb1".to_string())); mt.drop_table_by_id(DropTableByIdReq { if_exists: false, - tenant: tenant.to_string(), + tenant: req.name_ident.tenant.clone(), + db_id, + table_name: req.name_ident.table_name.clone(), tb_id: resp.table_id, }) .await?; @@ -3772,7 +3860,9 @@ impl SchemaApiTestSuite { drop_ids_2.push(DroppedId::Table(db_id, resp.table_id, "tb2".to_string())); mt.drop_table_by_id(DropTableByIdReq { if_exists: false, - tenant: tenant.to_string(), + tenant: req.name_ident.tenant.clone(), + db_id, + table_name: req.name_ident.table_name.clone(), tb_id: resp.table_id, }) .await?; @@ -3947,7 +4037,9 @@ impl SchemaApiTestSuite { mt.drop_table_by_id(DropTableByIdReq { if_exists: false, - tenant: tenant.to_string(), + tenant: req.name_ident.tenant.clone(), + db_id, + table_name: req.name_ident.table_name.clone(), tb_id: resp.table_id, }) .await?; @@ -4202,7 +4294,9 @@ impl SchemaApiTestSuite { let old_db = mt.get_database(Self::req_get_db(tenant, db_name)).await?; mt.drop_table_by_id(DropTableByIdReq { if_exists: false, - tenant: tenant.to_string(), + tenant: tbl_name_ident.tenant.clone(), + db_id: old_db.ident.db_id, + table_name: tbl_name_ident.table_name.clone(), tb_id, }) .await?; @@ -4258,6 +4352,8 @@ impl SchemaApiTestSuite { mt.drop_table_by_id(DropTableByIdReq { if_exists: false, tenant: tenant.to_string(), + db_id: old_db.ident.db_id, + table_name: tbl_name.to_string(), tb_id, }) .await?; @@ -4316,6 +4412,8 @@ impl SchemaApiTestSuite { mt.drop_table_by_id(DropTableByIdReq { if_exists: false, tenant: tenant.to_string(), + db_id: old_db.ident.db_id, + table_name: tbl_name.to_string(), tb_id: tb_info.ident.table_id, }) .await?; @@ -4418,6 +4516,8 @@ impl SchemaApiTestSuite { let drop_plan = DropTableByIdReq { if_exists: false, tenant: tenant.to_string(), + db_id: cur_db.ident.db_id, + table_name: tbl_name.to_string(), tb_id: new_tb_info.ident.table_id, }; @@ -6337,8 +6437,10 @@ where MT: SchemaApi + kvapi::AsKVApi async fn drop_table_by_id(&mut self) -> anyhow::Result<()> { let req = DropTableByIdReq { - if_exists: false, tenant: self.tenant(), + table_name: self.tbl_name(), + if_exists: false, + db_id: self.db_id, tb_id: self.table_id, }; self.mt.drop_table_by_id(req.clone()).await?; diff --git a/src/meta/api/src/share_api_test_suite.rs b/src/meta/api/src/share_api_test_suite.rs index b2bec9c20d33..011cb849a1cb 100644 --- a/src/meta/api/src/share_api_test_suite.rs +++ b/src/meta/api/src/share_api_test_suite.rs @@ -1705,7 +1705,9 @@ impl ShareApiTestSuite { let plan = DropTableByIdReq { if_exists: false, tenant: tenant.to_string(), + table_name: tbl_name.to_string(), tb_id: table_id, + db_id, }; let _res = mt.drop_table_by_id(plan).await; diff --git a/src/meta/app/src/schema/table.rs b/src/meta/app/src/schema/table.rs index 6a9870cca91f..4ddc658540ce 100644 --- a/src/meta/app/src/schema/table.rs +++ b/src/meta/app/src/schema/table.rs @@ -491,6 +491,10 @@ pub struct DropTableByIdReq { pub tenant: String, pub tb_id: MetaId, + + pub table_name: String, + + pub db_id: MetaId, } impl DropTableByIdReq { diff --git a/src/query/ee/src/stream/handler.rs b/src/query/ee/src/stream/handler.rs index 4cb0cc0c20d5..8c02f85bf52f 100644 --- a/src/query/ee/src/stream/handler.rs +++ b/src/query/ee/src/stream/handler.rs @@ -173,11 +173,15 @@ impl StreamHandler for RealStreamHandler { ))); } + let db = catalog.get_database(&tenant, &db_name).await?; + catalog .drop_table_by_id(DropTableByIdReq { if_exists: plan.if_exists, - tenant, + tenant: tenant.clone(), + table_name: stream_name.clone(), tb_id: table.get_id(), + db_id: db.get_db_info().ident.db_id, }) .await } else if plan.if_exists { diff --git a/src/query/service/src/interpreters/interpreter_table_drop.rs b/src/query/service/src/interpreters/interpreter_table_drop.rs index 1be50fc37590..60cb8c6e5bfc 100644 --- a/src/query/service/src/interpreters/interpreter_table_drop.rs +++ b/src/query/service/src/interpreters/interpreter_table_drop.rs @@ -107,8 +107,10 @@ impl Interpreter for DropTableInterpreter { let resp = catalog .drop_table_by_id(DropTableByIdReq { if_exists: self.plan.if_exists, - tenant: self.plan.tenant.clone(), + tenant, + table_name: tbl_name.to_string(), tb_id: tbl.get_table_info().ident.table_id, + db_id: db.get_db_info().ident.db_id, }) .await?; diff --git a/src/query/service/src/interpreters/interpreter_view_alter.rs b/src/query/service/src/interpreters/interpreter_view_alter.rs index 5781fc5d1bc0..56f7b0824069 100644 --- a/src/query/service/src/interpreters/interpreter_view_alter.rs +++ b/src/query/service/src/interpreters/interpreter_view_alter.rs @@ -55,11 +55,16 @@ impl Interpreter for AlterViewInterpreter { .get_table(&self.plan.tenant, &self.plan.database, &self.plan.view_name) .await { + let db = catalog + .get_database(&self.plan.tenant, &self.plan.database) + .await?; catalog .drop_table_by_id(DropTableByIdReq { if_exists: true, tenant: self.plan.tenant.clone(), + table_name: self.plan.view_name.clone(), tb_id: tbl.get_id(), + db_id: db.get_db_info().ident.db_id, }) .await?; diff --git a/src/query/service/src/interpreters/interpreter_view_drop.rs b/src/query/service/src/interpreters/interpreter_view_drop.rs index 1183c762cecd..079821f8ff8b 100644 --- a/src/query/service/src/interpreters/interpreter_view_drop.rs +++ b/src/query/service/src/interpreters/interpreter_view_drop.rs @@ -79,11 +79,16 @@ impl Interpreter for DropViewInterpreter { } let catalog = self.ctx.get_catalog(&self.plan.catalog).await?; + let db = catalog + .get_database(&self.plan.tenant, &self.plan.database) + .await?; catalog .drop_table_by_id(DropTableByIdReq { if_exists: self.plan.if_exists, tenant: self.plan.tenant.clone(), + table_name: self.plan.view_name.clone(), tb_id: table.get_id(), + db_id: db.get_db_info().ident.db_id, }) .await?; }; diff --git a/src/query/service/tests/it/catalogs/database_catalog.rs b/src/query/service/tests/it/catalogs/database_catalog.rs index 15eaae104eb8..ba289e6fa9ea 100644 --- a/src/query/service/tests/it/catalogs/database_catalog.rs +++ b/src/query/service/tests/it/catalogs/database_catalog.rs @@ -213,11 +213,14 @@ async fn test_catalogs_table() -> Result<()> { // Drop. { let tbl = catalog.get_table(tenant, "default", "test_table").await?; + let db = catalog.get_database(tenant, "default").await?; let res = catalog .drop_table_by_id(DropTableByIdReq { if_exists: false, tenant: tenant.to_string(), + table_name: "test_table".to_string(), tb_id: tbl.get_table_info().ident.table_id, + db_id: db.get_db_info().ident.db_id, }) .await; assert!(res.is_ok());