Skip to content

Commit a8ba76b

Browse files
mergify[bot]mergify[bot]
authored
Merge pull request #9250 from ariesdevil/bug-drop-current-db
fix: fix unknown database in query without relation to this database
2 parents d054cca + 4681918 commit a8ba76b

File tree

16 files changed

+121
-50
lines changed

16 files changed

+121
-50
lines changed

‎src/meta/api/src/schema_api_impl.rs

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ use common_meta_app::schema::DatabaseIdent;
3333
use common_meta_app::schema::DatabaseInfo;
3434
use common_meta_app::schema::DatabaseMeta;
3535
use common_meta_app::schema::DatabaseNameIdent;
36+
use common_meta_app::schema::DatabaseType;
3637
use common_meta_app::schema::DbIdList;
3738
use common_meta_app::schema::DbIdListKey;
3839
use common_meta_app::schema::DropDatabaseReply;
@@ -1602,6 +1603,11 @@ impl<KV: KVApi> SchemaApi for KV {
16021603
"get_table"
16031604
);
16041605

1606+
let db_type = match db_meta.from_share {
1607+
Some(share_ident) => DatabaseType::ShareDB(share_ident),
1608+
None => DatabaseType::NormalDB,
1609+
};
1610+
16051611
let tb_info = TableInfo {
16061612
ident: TableIdent {
16071613
table_id: tbid.table_id,
@@ -1612,7 +1618,7 @@ impl<KV: KVApi> SchemaApi for KV {
16121618
// Safe unwrap() because: tb_meta_seq > 0
16131619
meta: tb_meta.unwrap(),
16141620
tenant: req.tenant.clone(),
1615-
from_share: db_meta.from_share.clone(),
1621+
db_type,
16161622
};
16171623

16181624
return Ok(Arc::new(tb_info));
@@ -1698,6 +1704,11 @@ impl<KV: KVApi> SchemaApi for KV {
16981704
table_name: table_id_list_key.table_name.clone(),
16991705
};
17001706

1707+
let db_type = match db_meta.from_share.clone() {
1708+
Some(share_ident) => DatabaseType::ShareDB(share_ident),
1709+
None => DatabaseType::NormalDB,
1710+
};
1711+
17011712
let tb_info = TableInfo {
17021713
ident: TableIdent {
17031714
table_id: *table_id,
@@ -1707,7 +1718,7 @@ impl<KV: KVApi> SchemaApi for KV {
17071718
name: table_id_list_key.table_name.clone(),
17081719
meta: tb_meta,
17091720
tenant: tenant_dbname.tenant.clone(),
1710-
from_share: db_meta.from_share.clone(),
1721+
db_type,
17111722
};
17121723

17131724
tb_info_list.push(Arc::new(tb_info));
@@ -2633,7 +2644,7 @@ async fn get_tableinfos_by_ids(
26332644
ids: &[u64],
26342645
tenant_dbname: &DatabaseNameIdent,
26352646
dbid_tbnames_opt: Option<Vec<DBIdTableName>>,
2636-
share_name: Option<ShareNameIdent>,
2647+
db_type: DatabaseType,
26372648
) -> Result<Vec<Arc<TableInfo>>, KVAppError> {
26382649
let mut tb_meta_keys = Vec::with_capacity(ids.len());
26392650
for id in ids.iter() {
@@ -2671,7 +2682,7 @@ async fn get_tableinfos_by_ids(
26712682
meta: tb_meta,
26722683
name: tbnames[i].clone(),
26732684
tenant: tenant_dbname.tenant.clone(),
2674-
from_share: share_name.clone(),
2685+
db_type: db_type.clone(),
26752686
};
26762687
tb_infos.push(Arc::new(tb_info));
26772688
} else {
@@ -2700,7 +2711,14 @@ async fn list_tables_from_unshare_db(
27002711

27012712
let (dbid_tbnames, ids) = list_u64_value(kv_api, &dbid_tbname).await?;
27022713

2703-
get_tableinfos_by_ids(kv_api, &ids, tenant_dbname, Some(dbid_tbnames), None).await
2714+
get_tableinfos_by_ids(
2715+
kv_api,
2716+
&ids,
2717+
tenant_dbname,
2718+
Some(dbid_tbnames),
2719+
DatabaseType::NormalDB,
2720+
)
2721+
.await
27042722
}
27052723

27062724
async fn list_tables_from_share_db(
@@ -2739,5 +2757,12 @@ async fn list_tables_from_share_db(
27392757
ids.push(table_id);
27402758
}
27412759
}
2742-
get_tableinfos_by_ids(kv_api, &ids, tenant_dbname, None, Some(share)).await
2760+
get_tableinfos_by_ids(
2761+
kv_api,
2762+
&ids,
2763+
tenant_dbname,
2764+
None,
2765+
DatabaseType::ShareDB(share),
2766+
)
2767+
.await
27432768
}

‎src/meta/api/src/schema_api_test_suite.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ use common_meta_app::schema::DatabaseIdToName;
3030
use common_meta_app::schema::DatabaseInfo;
3131
use common_meta_app::schema::DatabaseMeta;
3232
use common_meta_app::schema::DatabaseNameIdent;
33+
use common_meta_app::schema::DatabaseType;
3334
use common_meta_app::schema::DbIdList;
3435
use common_meta_app::schema::DbIdListKey;
3536
use common_meta_app::schema::DropDatabaseReq;
@@ -3533,7 +3534,7 @@ impl SchemaApiTestSuite {
35333534
assert_eq!(table_info.name, tb1.to_string());
35343535
assert_eq!(table_info.ident.table_id, share_table_id);
35353536
assert_eq!(table_info.tenant, tenant2.to_string());
3536-
assert_eq!(table_info.from_share, Some(share_name.clone()));
3537+
assert_eq!(table_info.db_type, DatabaseType::ShareDB(share_name));
35373538
}
35383539

35393540
info!("--- get tables from share db");

‎src/meta/app/src/schema/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ pub use table::CountTablesReq;
4747
pub use table::CreateTableReply;
4848
pub use table::CreateTableReq;
4949
pub use table::DBIdTableName;
50+
pub use table::DatabaseType;
5051
pub use table::DropTableReply;
5152
pub use table::DropTableReq;
5253
pub use table::GetTableCopiedFileReply;

‎src/meta/app/src/schema/table.rs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,30 @@ impl Display for TableIdListKey {
135135
}
136136
}
137137

138+
#[derive(serde::Serialize, serde::Deserialize, Clone, Debug, Eq, PartialEq, Default)]
139+
pub enum DatabaseType {
140+
#[default]
141+
NormalDB,
142+
ShareDB(ShareNameIdent),
143+
}
144+
145+
impl Display for DatabaseType {
146+
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
147+
match self {
148+
DatabaseType::NormalDB => {
149+
write!(f, "normal database")
150+
}
151+
DatabaseType::ShareDB(share_ident) => {
152+
write!(
153+
f,
154+
"share database: {}-{}",
155+
share_ident.tenant, share_ident.share_name
156+
)
157+
}
158+
}
159+
}
160+
}
161+
138162
#[derive(serde::Serialize, serde::Deserialize, Clone, Debug, Eq, PartialEq, Default)]
139163
pub struct TableInfo {
140164
pub ident: TableIdent,
@@ -156,8 +180,8 @@ pub struct TableInfo {
156180

157181
pub tenant: String,
158182

159-
// If not None, means that the table is share from other tenant.
160-
pub from_share: Option<ShareNameIdent>,
183+
// table belong to which type of database.
184+
pub db_type: DatabaseType,
161185
}
162186

163187
#[derive(serde::Serialize, serde::Deserialize, Clone, Debug, Eq, PartialEq, Default)]

‎src/query/catalog/src/catalog/interface.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,7 @@ pub trait Catalog: DynClone + Send + Sync {
142142

143143
async fn update_table_meta(
144144
&self,
145-
tenant: &str,
146-
db_name: &str,
145+
table_info: &TableInfo,
147146
req: UpdateTableMetaReq,
148147
) -> Result<UpdateTableMetaReply>;
149148

‎src/query/catalog/src/table.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ use common_datavalues::DataSchemaRef;
2424
use common_datavalues::DataValue;
2525
use common_exception::ErrorCode;
2626
use common_exception::Result;
27+
use common_meta_app::schema::DatabaseType;
2728
use common_meta_app::schema::TableInfo;
2829
use common_meta_types::MetaId;
2930
use common_pipeline_core::Pipeline;
@@ -311,7 +312,7 @@ pub trait TableExt: Table {
311312
name,
312313
meta: meta.as_ref().clone(),
313314
tenant: "".to_owned(),
314-
from_share: None,
315+
db_type: DatabaseType::NormalDB,
315316
};
316317
catalog.get_table_by_info(&table_info)
317318
}

‎src/query/service/src/catalogs/default/database_catalog.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -461,12 +461,11 @@ impl Catalog for DatabaseCatalog {
461461

462462
async fn update_table_meta(
463463
&self,
464-
tenant: &str,
465-
db_name: &str,
464+
table_info: &TableInfo,
466465
req: UpdateTableMetaReq,
467466
) -> Result<UpdateTableMetaReply> {
468467
self.mutable_catalog
469-
.update_table_meta(tenant, db_name, req)
468+
.update_table_meta(table_info, req)
470469
.await
471470
}
472471

‎src/query/service/src/catalogs/default/immutable_catalog.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,8 +248,7 @@ impl Catalog for ImmutableCatalog {
248248

249249
async fn update_table_meta(
250250
&self,
251-
_tenant: &str,
252-
_db_name: &str,
251+
_table_info: &TableInfo,
253252
req: UpdateTableMetaReq,
254253
) -> Result<UpdateTableMetaReply> {
255254
Err(ErrorCode::Unimplemented(format!(

‎src/query/service/src/catalogs/default/mutable_catalog.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ use common_meta_app::schema::DatabaseIdent;
2727
use common_meta_app::schema::DatabaseInfo;
2828
use common_meta_app::schema::DatabaseMeta;
2929
use common_meta_app::schema::DatabaseNameIdent;
30+
use common_meta_app::schema::DatabaseType;
3031
use common_meta_app::schema::DropDatabaseReq;
3132
use common_meta_app::schema::DropTableReply;
3233
use common_meta_app::schema::DropTableReq;
@@ -281,12 +282,18 @@ impl Catalog for MutableCatalog {
281282

282283
async fn update_table_meta(
283284
&self,
284-
tenant: &str,
285-
db_name: &str,
285+
table_info: &TableInfo,
286286
req: UpdateTableMetaReq,
287287
) -> Result<UpdateTableMetaReply> {
288-
let db = self.get_database(tenant, db_name).await?;
289-
db.update_table_meta(req).await
288+
match table_info.db_type.clone() {
289+
DatabaseType::NormalDB => Ok(self.ctx.meta.update_table_meta(req).await?),
290+
DatabaseType::ShareDB(share_ident) => {
291+
let db = self
292+
.get_database(&share_ident.tenant, &share_ident.share_name)
293+
.await?;
294+
db.update_table_meta(req).await
295+
}
296+
}
290297
}
291298

292299
async fn get_table_copied_file_info(

‎src/query/service/tests/it/storages/fuse/operations/commit.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -543,14 +543,13 @@ impl Catalog for FakedCatalog {
543543

544544
async fn update_table_meta(
545545
&self,
546-
tenant: &str,
547-
db_name: &str,
546+
table_info: &TableInfo,
548547
req: UpdateTableMetaReq,
549548
) -> Result<UpdateTableMetaReply> {
550549
if let Some(e) = &self.error_injection {
551550
Err(e.clone())
552551
} else {
553-
self.cat.update_table_meta(tenant, db_name, req).await
552+
self.cat.update_table_meta(table_info, req).await
554553
}
555554
}
556555

0 commit comments

Comments
 (0)