Skip to content

Commit 13e59ec

Browse files
drmingdrmerdrmingdrmer
committed
refactor: Define DatabaseIdToNameIdent with TIdent
1 parent c221094 commit 13e59ec

File tree

17 files changed

+164
-96
lines changed

17 files changed

+164
-96
lines changed

‎Cargo.lock

-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/meta/api/src/schema_api.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use databend_common_meta_app::schema::CreateTableReply;
2929
use databend_common_meta_app::schema::CreateTableReq;
3030
use databend_common_meta_app::schema::CreateVirtualColumnReply;
3131
use databend_common_meta_app::schema::CreateVirtualColumnReq;
32+
use databend_common_meta_app::schema::DatabaseIdIdent;
3233
use databend_common_meta_app::schema::DatabaseInfo;
3334
use databend_common_meta_app::schema::DeleteLockRevReq;
3435
use databend_common_meta_app::schema::DropCatalogReply;
@@ -223,7 +224,7 @@ pub trait SchemaApi: Send + Sync {
223224
db_ids: &[MetaId],
224225
) -> Result<Vec<Option<String>>, KVAppError>;
225226

226-
async fn get_db_name_by_id(&self, db_id: MetaId) -> Result<String, KVAppError>;
227+
async fn get_db_name_by_id(&self, db_id: DatabaseIdIdent) -> Result<String, KVAppError>;
227228

228229
async fn get_table_copied_file_info(
229230
&self,

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

+12-11
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ use databend_common_meta_app::schema::CreateVirtualColumnReq;
8585
use databend_common_meta_app::schema::DBIdTableName;
8686
use databend_common_meta_app::schema::DatabaseIdHistoryIdent;
8787
use databend_common_meta_app::schema::DatabaseIdIdent;
88-
use databend_common_meta_app::schema::DatabaseIdToName;
88+
use databend_common_meta_app::schema::DatabaseIdToNameIdent;
8989
use databend_common_meta_app::schema::DatabaseIdent;
9090
use databend_common_meta_app::schema::DatabaseInfo;
9191
use databend_common_meta_app::schema::DatabaseInfoFilter;
@@ -322,7 +322,7 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
322322

323323
let db_id = fetch_id(self, IdGenerator::database_id()).await?;
324324
let db_id_ident = DatabaseIdIdent::new(&tenant, db_id);
325-
let id_to_name_key = DatabaseIdToName { db_id };
325+
let id_to_name_key = DatabaseIdToNameIdent::new(&tenant, db_id);
326326

327327
debug!(db_id = db_id, name_key :? =(name_key); "new database id");
328328

@@ -520,6 +520,7 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
520520
) -> Result<RenameDatabaseReply, KVAppError> {
521521
debug!(req :? =(&req); "SchemaApi: {}", func_name!());
522522

523+
let tenant = req.name_ident.tenant().clone();
523524
let tenant_dbname = &req.name_ident;
524525
let tenant_newdbname = DatabaseNameIdent::new(tenant_dbname.tenant(), &req.new_db_name);
525526

@@ -548,7 +549,7 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
548549
db_has_to_not_exist(db_id_seq, &tenant_newdbname, "rename_database")?;
549550

550551
// get db id -> name
551-
let db_id_key = DatabaseIdToName { db_id: old_db_id };
552+
let db_id_key = DatabaseIdToNameIdent::new(&tenant, old_db_id);
552553
let (db_name_seq, _): (_, Option<DatabaseNameIdentRaw>) =
553554
get_pb_value(self, &db_id_key).await?;
554555

@@ -2262,19 +2263,19 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
22622263

22632264
#[logcall::logcall("debug")]
22642265
#[minitrace::trace]
2265-
async fn get_db_name_by_id(&self, db_id: u64) -> Result<String, KVAppError> {
2266-
debug!(req :? =(&db_id); "SchemaApi: {}", func_name!());
2266+
async fn get_db_name_by_id(&self, db_id_ident: DatabaseIdIdent) -> Result<String, KVAppError> {
2267+
debug!(req :? =(&db_id_ident); "SchemaApi: {}", func_name!());
22672268

2268-
let db_id_to_name_key = DatabaseIdToName { db_id };
2269+
let db_id_to_name_ident = DatabaseIdToNameIdent::new_from(db_id_ident.clone());
22692270

22702271
let (meta_seq, db_name): (_, Option<DatabaseNameIdentRaw>) =
2271-
get_pb_value(self, &db_id_to_name_key).await?;
2272+
get_pb_value(self, &db_id_to_name_ident).await?;
22722273

2273-
debug!(ident :% =(&db_id_to_name_key); "get_db_name_by_id");
2274+
debug!(ident :% =(&db_id_to_name_ident.display()); "get_db_name_by_id");
22742275

22752276
if meta_seq == 0 || db_name.is_none() {
22762277
return Err(KVAppError::AppError(AppError::UnknownDatabaseId(
2277-
UnknownDatabaseId::new(db_id, "get_db_name_by_id"),
2278+
UnknownDatabaseId::new(db_id_ident.database_id(), "get_db_name_by_id"),
22782279
)));
22792280
}
22802281

@@ -2292,7 +2293,7 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
22922293

22932294
let mut kv_keys = Vec::with_capacity(db_ids.len());
22942295
for id in db_ids {
2295-
let k = DatabaseIdToName { db_id: *id }.to_string_key();
2296+
let k = DatabaseIdToNameIdent::new(tenant, *id).to_string_key();
22962297
kv_keys.push(k);
22972298
}
22982299

@@ -4684,7 +4685,7 @@ async fn gc_dropped_db_by_id(
46844685
if db_meta_seq == 0 {
46854686
return Ok(());
46864687
}
4687-
let id_to_name = DatabaseIdToName { db_id };
4688+
let id_to_name = DatabaseIdToNameIdent::new(tenant, db_id);
46884689
let (name_ident_seq, _name_ident): (_, Option<DatabaseNameIdentRaw>) =
46894690
get_pb_value(kv_api, &id_to_name).await?;
46904691
if name_ident_seq == 0 {

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

+17-11
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ use databend_common_meta_app::schema::CreateVirtualColumnReq;
5252
use databend_common_meta_app::schema::DBIdTableName;
5353
use databend_common_meta_app::schema::DatabaseIdHistoryIdent;
5454
use databend_common_meta_app::schema::DatabaseIdIdent;
55-
use databend_common_meta_app::schema::DatabaseIdToName;
55+
use databend_common_meta_app::schema::DatabaseIdToNameIdent;
5656
use databend_common_meta_app::schema::DatabaseInfo;
5757
use databend_common_meta_app::schema::DatabaseMeta;
5858
use databend_common_meta_app::schema::DbIdList;
@@ -470,7 +470,7 @@ impl SchemaApiTestSuite {
470470
let res = mt.create_table(req).await?;
471471
table_id = res.table_id;
472472

473-
let db_id_name_key = DatabaseIdToName { db_id };
473+
let db_id_name_key = DatabaseIdToNameIdent::new(&tenant, db_id);
474474
let ret_db_name_ident: DatabaseNameIdentRaw =
475475
get_kv_data(mt.as_kv_api(), &db_id_name_key).await?;
476476
assert_eq!(
@@ -498,7 +498,7 @@ impl SchemaApiTestSuite {
498498
info!("rename database res: {:?}", res);
499499
assert!(res.is_ok());
500500

501-
let db_id_2_name_key = DatabaseIdToName { db_id };
501+
let db_id_2_name_key = DatabaseIdToNameIdent::new(&tenant, db_id);
502502
let ret_db_name_ident: DatabaseNameIdentRaw =
503503
get_kv_data(mt.as_kv_api(), &db_id_2_name_key).await?;
504504
assert_eq!(
@@ -724,7 +724,7 @@ impl SchemaApiTestSuite {
724724

725725
let orig_db_id: u64 = get_kv_u64_data(mt.as_kv_api(), &db_name).await?;
726726
assert_eq!(orig_db_id, res.ident.db_id);
727-
let db_id_name_key = DatabaseIdToName { db_id: orig_db_id };
727+
let db_id_name_key = DatabaseIdToNameIdent::new(&tenant, orig_db_id);
728728
let ret_db_name_ident: DatabaseNameIdentRaw =
729729
get_kv_data(mt.as_kv_api(), &db_id_name_key).await?;
730730
assert_eq!(ret_db_name_ident, DatabaseNameIdentRaw::from(&db_name));
@@ -748,7 +748,7 @@ impl SchemaApiTestSuite {
748748

749749
let db_id: u64 = get_kv_u64_data(mt.as_kv_api(), &db_name).await?;
750750
assert_eq!(db_id, res.ident.db_id);
751-
let db_id_name_key = DatabaseIdToName { db_id };
751+
let db_id_name_key = DatabaseIdToNameIdent::new(&tenant, db_id);
752752
let ret_db_name_ident: DatabaseNameIdentRaw =
753753
get_kv_data(mt.as_kv_api(), &db_id_name_key).await?;
754754
assert_eq!(ret_db_name_ident, DatabaseNameIdentRaw::from(&db_name));
@@ -2038,7 +2038,7 @@ impl SchemaApiTestSuite {
20382038

20392039
info!("--- drop db-id-to-name mapping to ensure dropping table does not rely on it");
20402040
{
2041-
let id_to_name_key = DatabaseIdToName { db_id: util.db_id };
2041+
let id_to_name_key = DatabaseIdToNameIdent::new(&util.tenant, util.db_id);
20422042
util.mt
20432043
.as_kv_api()
20442044
.upsert_kv(UpsertKV::delete(id_to_name_key.to_string_key()))
@@ -3384,7 +3384,7 @@ impl SchemaApiTestSuite {
33843384
// assert old db meta and id to name mapping has been removed
33853385
for db_id in old_id_list.iter() {
33863386
let id_key = DatabaseIdIdent::new(&tenant, *db_id);
3387-
let id_mapping = DatabaseIdToName { db_id: *db_id };
3387+
let id_mapping = DatabaseIdToNameIdent::new(&tenant, *db_id);
33883388

33893389
let meta_res: Result<DatabaseMeta, KVAppError> =
33903390
get_kv_data(mt.as_kv_api(), &id_key).await;
@@ -3767,7 +3767,7 @@ impl SchemaApiTestSuite {
37673767
// assert old db meta and id to name mapping has been removed
37683768
for db_id in old_id_list.id_list.iter() {
37693769
let id_key = DatabaseIdIdent::new(&tenant, *db_id);
3770-
let id_mapping = DatabaseIdToName { db_id: *db_id };
3770+
let id_mapping = DatabaseIdToNameIdent::new(&tenant, *db_id);
37713771

37723772
let meta_res: Result<DatabaseMeta, KVAppError> =
37733773
get_kv_data(mt.as_kv_api(), &id_key).await;
@@ -4981,7 +4981,9 @@ impl SchemaApiTestSuite {
49814981

49824982
assert_eq!(1, res.db_id, "first database id is 1");
49834983

4984-
let got = mt.get_db_name_by_id(res.db_id).await?;
4984+
let got = mt
4985+
.get_db_name_by_id(DatabaseIdIdent::new(&tenant, res.db_id))
4986+
.await?;
49854987
assert_eq!(got, db_name.to_string())
49864988
}
49874989

@@ -4995,14 +4997,18 @@ impl SchemaApiTestSuite {
49954997

49964998
let db = mt.get_database(plan).await.unwrap();
49974999

4998-
let got = mt.get_db_name_by_id(db.ident.db_id).await?;
5000+
let got = mt
5001+
.get_db_name_by_id(DatabaseIdIdent::new(&tenant, db.ident.db_id))
5002+
.await?;
49995003

50005004
assert_eq!(got, db_name.to_string());
50015005
}
50025006

50035007
info!("--- get_db_name_by_id with not exists db_id");
50045008
{
5005-
let got = mt.get_db_name_by_id(1024).await;
5009+
let got = mt
5010+
.get_db_name_by_id(DatabaseIdIdent::new(&tenant, 1024))
5011+
.await;
50065012

50075013
let err = got.unwrap_err();
50085014
let err = ErrorCode::from(err);

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

-55
Original file line numberDiff line numberDiff line change
@@ -42,23 +42,6 @@ pub struct DatabaseIdent {
4242
pub seq: u64,
4343
}
4444

45-
#[derive(Clone, Debug, Default, Eq, PartialEq)]
46-
pub struct DatabaseIdToName {
47-
pub db_id: u64,
48-
}
49-
50-
impl Display for DatabaseIdToName {
51-
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
52-
write!(f, "{}", self.db_id)
53-
}
54-
}
55-
56-
impl DatabaseIdToName {
57-
pub fn new(db_id: u64) -> Self {
58-
DatabaseIdToName { db_id }
59-
}
60-
}
61-
6245
#[derive(Clone, Debug, Eq, PartialEq)]
6346
pub struct DatabaseMeta {
6447
pub engine: String,
@@ -299,41 +282,3 @@ impl ListDatabaseReq {
299282
&self.tenant
300283
}
301284
}
302-
303-
mod kvapi_key_impl {
304-
use databend_common_meta_kvapi::kvapi;
305-
306-
use crate::schema::database_name_ident::DatabaseNameIdentRaw;
307-
use crate::schema::DatabaseIdIdent;
308-
use crate::schema::DatabaseIdToName;
309-
use crate::tenant::Tenant;
310-
311-
impl kvapi::KeyCodec for DatabaseIdToName {
312-
fn encode_key(&self, b: kvapi::KeyBuilder) -> kvapi::KeyBuilder {
313-
b.push_u64(self.db_id)
314-
}
315-
316-
fn decode_key(parser: &mut kvapi::KeyParser) -> Result<Self, kvapi::KeyError> {
317-
let db_id = parser.next_u64()?;
318-
Ok(Self { db_id })
319-
}
320-
}
321-
322-
/// "__fd_database_id_to_name/<db_id> -> DatabaseNameIdent"
323-
impl kvapi::Key for DatabaseIdToName {
324-
const PREFIX: &'static str = "__fd_database_id_to_name";
325-
326-
type ValueType = DatabaseNameIdentRaw;
327-
328-
fn parent(&self) -> Option<String> {
329-
// TODO(TIdent): add real tenant
330-
Some(DatabaseIdIdent::new(Tenant::new_literal("dummy"), self.db_id).to_string_key())
331-
}
332-
}
333-
334-
impl kvapi::Value for DatabaseNameIdentRaw {
335-
fn dependency_keys(&self) -> impl IntoIterator<Item = String> {
336-
[]
337-
}
338-
}
339-
}

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

+91
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// Copyright 2021 Datafuse Labs
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
use crate::tenant_key::ident::TIdent;
16+
use crate::tenant_key::raw::TIdentRaw;
17+
18+
pub type DatabaseIdToNameIdent = TIdent<Resource, u64>;
19+
pub type DatabaseIdToNameIdentRaw = TIdentRaw<Resource, u64>;
20+
21+
pub use kvapi_impl::Resource;
22+
23+
impl DatabaseIdToNameIdent {
24+
pub fn database_id(&self) -> u64 {
25+
*self.name()
26+
}
27+
}
28+
29+
impl DatabaseIdToNameIdentRaw {
30+
pub fn database_id(&self) -> u64 {
31+
*self.name()
32+
}
33+
}
34+
35+
mod kvapi_impl {
36+
37+
use databend_common_meta_kvapi::kvapi;
38+
39+
use crate::schema::database_name_ident::DatabaseNameIdentRaw;
40+
use crate::tenant_key::resource::TenantResource;
41+
42+
pub struct Resource;
43+
impl TenantResource for Resource {
44+
const PREFIX: &'static str = "__fd_database_id_to_name";
45+
const TYPE: &'static str = "DatabaseIdToNameIdent";
46+
const HAS_TENANT: bool = false;
47+
type ValueType = DatabaseNameIdentRaw;
48+
}
49+
50+
// TODO(TIdent): parent: DatabaseIdIdent
51+
impl kvapi::Value for DatabaseNameIdentRaw {
52+
fn dependency_keys(&self) -> impl IntoIterator<Item = String> {
53+
[]
54+
}
55+
}
56+
57+
// // Use these error types to replace usage of ErrorCode if possible.
58+
// impl From<ExistError<Resource>> for ErrorCode {
59+
// impl From<UnknownError<Resource>> for ErrorCode {
60+
}
61+
62+
#[cfg(test)]
63+
mod tests {
64+
use databend_common_meta_kvapi::kvapi::Key;
65+
66+
use super::DatabaseIdToNameIdent;
67+
use crate::tenant::Tenant;
68+
69+
#[test]
70+
fn test_background_job_id_ident() {
71+
let tenant = Tenant::new_literal("dummy");
72+
let ident = DatabaseIdToNameIdent::new(tenant, 3);
73+
74+
let key = ident.to_string_key();
75+
assert_eq!(key, "__fd_database_id_to_name/3");
76+
77+
assert_eq!(ident, DatabaseIdToNameIdent::from_str_key(&key).unwrap());
78+
}
79+
80+
#[test]
81+
fn test_background_job_id_ident_with_key_space() {
82+
// TODO(xp): implement this test
83+
// let tenant = Tenant::new_literal("test");
84+
// let ident = DatabaseIdToNameIdent::new(tenant, 3);
85+
//
86+
// let key = ident.to_string_key();
87+
// assert_eq!(key, "__fd_database_id_to_name/3");
88+
//
89+
// assert_eq!(ident, DatabaseIdToNameIdent::from_str_key(&key).unwrap());
90+
}
91+
}

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ pub mod catalog_id_to_name_ident;
2020
pub mod catalog_name_ident;
2121
pub mod database_id_history_ident;
2222
pub mod database_id_ident;
23+
pub mod database_id_to_name_ident;
2324
pub mod database_name_ident;
2425
pub mod index_name_ident;
2526
pub mod table_lock_ident;
@@ -42,7 +43,6 @@ pub use catalog_name_ident::CatalogNameIdent;
4243
pub use create_option::CreateOption;
4344
pub use database::CreateDatabaseReply;
4445
pub use database::CreateDatabaseReq;
45-
pub use database::DatabaseIdToName;
4646
pub use database::DatabaseIdent;
4747
pub use database::DatabaseInfo;
4848
pub use database::DatabaseInfoFilter;
@@ -58,6 +58,7 @@ pub use database::UndropDatabaseReply;
5858
pub use database::UndropDatabaseReq;
5959
pub use database_id_history_ident::DatabaseIdHistoryIdent;
6060
pub use database_id_ident::DatabaseIdIdent;
61+
pub use database_id_to_name_ident::DatabaseIdToNameIdent;
6162
pub use index::*;
6263
pub use index_name_ident::IndexNameIdent;
6364
pub use index_name_ident::IndexNameIdentRaw;

0 commit comments

Comments
 (0)