Skip to content

Commit d799038

Browse files
authored
refactor: define CatalogIdIdent with TIdent (#15429)
1 parent 799cb12 commit d799038

File tree

7 files changed

+116
-70
lines changed

7 files changed

+116
-70
lines changed

src/meta/api/src/schema_api_impl.rs

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ use databend_common_meta_app::data_mask::MaskpolicyTableIdList;
6464
use databend_common_meta_app::id_generator::IdGenerator;
6565
use databend_common_meta_app::schema::database_name_ident::DatabaseNameIdent;
6666
use databend_common_meta_app::schema::database_name_ident::DatabaseNameIdentRaw;
67-
use databend_common_meta_app::schema::CatalogId;
67+
use databend_common_meta_app::schema::CatalogIdIdent;
6868
use databend_common_meta_app::schema::CatalogIdToName;
6969
use databend_common_meta_app::schema::CatalogInfo;
7070
use databend_common_meta_app::schema::CatalogMeta;
@@ -3669,7 +3669,7 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
36693669
// (catalog_id) -> catalog_meta
36703670
// (catalog_id) -> (tenant, catalog_name)
36713671
let catalog_id = fetch_id(self, IdGenerator::catalog_id()).await?;
3672-
let id_key = CatalogId { catalog_id };
3672+
let id_key = CatalogIdIdent::new(name_key.tenant(), catalog_id);
36733673
let id_to_name_key = CatalogIdToName { catalog_id };
36743674

36753675
debug!(catalog_id = catalog_id, name_key :? =(name_key); "new catalog id");
@@ -3718,7 +3718,7 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
37183718
get_catalog_or_err(self, name_key, "get_catalog").await?;
37193719

37203720
let catalog = CatalogInfo {
3721-
id: CatalogId { catalog_id }.into(),
3721+
id: CatalogIdIdent::new(name_key.tenant(), catalog_id).into(),
37223722
name_ident: name_key.clone().into(),
37233723
meta: catalog_meta,
37243724
};
@@ -3761,7 +3761,7 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
37613761
// (tenant, catalog_name) -> catalog_id
37623762
// (catalog_id) -> catalog_meta
37633763
// (catalog_id) -> (tenant, catalog_name)
3764-
let id_key = CatalogId { catalog_id };
3764+
let id_key = CatalogIdIdent::new(name_key.tenant(), catalog_id);
37653765
let id_to_name_key = CatalogIdToName { catalog_id };
37663766

37673767
debug!(
@@ -3811,7 +3811,7 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
38113811
debug!(req :? =(&req); "SchemaApi: {}", func_name!());
38123812

38133813
let tenant = req.tenant;
3814-
let name_key = CatalogNameIdent::new(tenant, "");
3814+
let name_key = CatalogNameIdent::new(&tenant, "");
38153815

38163816
// Pairs of catalog-name and catalog_id with seq
38173817
let (tenant_catalog_names, catalog_ids) = list_u64_value(self, &name_key).await?;
@@ -3820,10 +3820,7 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
38203820
let mut kv_keys = Vec::with_capacity(catalog_ids.len());
38213821

38223822
for catalog_id in catalog_ids.iter() {
3823-
let k = CatalogId {
3824-
catalog_id: *catalog_id,
3825-
}
3826-
.to_string_key();
3823+
let k = CatalogIdIdent::new(&tenant, *catalog_id).to_string_key();
38273824
kv_keys.push(k);
38283825
}
38293826

@@ -3838,10 +3835,7 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
38383835
let catalog_meta: CatalogMeta = deserialize_struct(&seq_meta.data)?;
38393836

38403837
let catalog_info = CatalogInfo {
3841-
id: CatalogId {
3842-
catalog_id: catalog_ids[i],
3843-
}
3844-
.into(),
3838+
id: CatalogIdIdent::new(&tenant, catalog_ids[i]).into(),
38453839
name_ident: CatalogNameIdent::new(
38463840
name_key.tenant().clone(),
38473841
tenant_catalog_names[i].name(),
@@ -5167,7 +5161,7 @@ pub(crate) async fn get_catalog_or_err(
51675161
let (catalog_id_seq, catalog_id) = get_u64_value(kv_api, name_key).await?;
51685162
catalog_has_to_exist(catalog_id_seq, name_key, &msg)?;
51695163

5170-
let id_key = CatalogId { catalog_id };
5164+
let id_key = CatalogIdIdent::new(name_key.tenant(), catalog_id);
51715165

51725166
let (catalog_meta_seq, catalog_meta) = get_pb_value(kv_api, &id_key).await?;
51735167
catalog_has_to_exist(catalog_meta_seq, name_key, msg)?;

src/meta/app/src/schema/catalog.rs

Lines changed: 8 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use std::ops::Deref;
1818
use chrono::DateTime;
1919
use chrono::Utc;
2020

21+
use crate::schema::catalog::catalog_info::CatalogId;
2122
use crate::schema::CatalogNameIdent;
2223
use crate::storage::StorageParams;
2324
use crate::tenant::Tenant;
@@ -112,7 +113,7 @@ pub struct CatalogInfo {
112113
/// Private types for `CatalogInfo`.
113114
mod catalog_info {
114115

115-
/// Same as [`crate::schema::CatalogId`], except with serde support, and can be used in a value,
116+
/// Same as [`crate::schema::CatalogIdIdent`], except with serde support, and can be used in a value,
116117
/// while CatalogId is only used for Key.
117118
///
118119
/// This type is sealed in a private mod so that it is pub for use but can not be created directly.
@@ -121,10 +122,10 @@ mod catalog_info {
121122
pub catalog_id: u64,
122123
}
123124

124-
impl From<crate::schema::CatalogId> for CatalogId {
125-
fn from(value: crate::schema::CatalogId) -> Self {
125+
impl From<crate::schema::CatalogIdIdent> for CatalogId {
126+
fn from(value: crate::schema::CatalogIdIdent) -> Self {
126127
Self {
127-
catalog_id: value.catalog_id,
128+
catalog_id: value.catalog_id(),
128129
}
129130
}
130131
}
@@ -144,7 +145,7 @@ impl CatalogInfo {
144145
/// Create a new default catalog info.
145146
pub fn new_default() -> CatalogInfo {
146147
Self {
147-
id: CatalogId { catalog_id: 0 }.into(),
148+
id: CatalogId { catalog_id: 0 },
148149
name_ident: CatalogName {
149150
// tenant for default catalog is not used.
150151
tenant: "".to_string(),
@@ -164,18 +165,6 @@ pub struct CatalogMeta {
164165
pub created_on: DateTime<Utc>,
165166
}
166167

167-
// serde is required by `CatalogInfo.id`
168-
#[derive(Clone, Debug, Default, Eq, PartialEq)]
169-
pub struct CatalogId {
170-
pub catalog_id: u64,
171-
}
172-
173-
impl CatalogId {
174-
pub fn new(catalog_id: u64) -> CatalogId {
175-
CatalogId { catalog_id }
176-
}
177-
}
178-
179168
#[derive(Clone, Debug, Default, Eq, PartialEq)]
180169
pub struct CatalogIdToName {
181170
pub catalog_id: u64,
@@ -278,34 +267,9 @@ mod kvapi_key_impl {
278267
use databend_common_meta_kvapi::kvapi::KeyError;
279268
use databend_common_meta_kvapi::kvapi::KeyParser;
280269

281-
use super::CatalogId;
282270
use super::CatalogIdToName;
283-
use crate::schema::CatalogMeta;
284271
use crate::schema::CatalogNameIdent;
285272

286-
impl kvapi::KeyCodec for CatalogId {
287-
fn encode_key(&self, b: KeyBuilder) -> KeyBuilder {
288-
b.push_u64(self.catalog_id)
289-
}
290-
291-
fn decode_key(parser: &mut KeyParser) -> Result<Self, KeyError> {
292-
let catalog_id = parser.next_u64()?;
293-
294-
Ok(Self { catalog_id })
295-
}
296-
}
297-
298-
/// "__fd_catalog_by_id/<catalog_id>"
299-
impl kvapi::Key for CatalogId {
300-
const PREFIX: &'static str = "__fd_catalog_by_id";
301-
302-
type ValueType = CatalogMeta;
303-
304-
fn parent(&self) -> Option<String> {
305-
None
306-
}
307-
}
308-
309273
impl kvapi::KeyCodec for CatalogIdToName {
310274
fn encode_key(&self, b: KeyBuilder) -> KeyBuilder {
311275
b.push_u64(self.catalog_id)
@@ -325,13 +289,8 @@ mod kvapi_key_impl {
325289
type ValueType = CatalogNameIdent;
326290

327291
fn parent(&self) -> Option<String> {
328-
Some(CatalogId::new(self.catalog_id).to_string_key())
329-
}
330-
}
331-
332-
impl kvapi::Value for CatalogMeta {
333-
fn dependency_keys(&self) -> impl IntoIterator<Item = String> {
334-
[]
292+
None
293+
// Some(CatalogIdIdent::new(self.catalog_id).to_string_key())
335294
}
336295
}
337296

src/meta/app/src/schema/catalog_id.rs

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
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 CatalogIdIdent = TIdent<Resource, u64>;
19+
pub type CatalogIdIdentRaw = TIdentRaw<Resource, u64>;
20+
21+
pub use kvapi_impl::Resource;
22+
23+
impl CatalogIdIdent {
24+
pub fn catalog_id(&self) -> u64 {
25+
*self.name()
26+
}
27+
}
28+
29+
impl CatalogIdIdentRaw {
30+
pub fn catalog_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::CatalogMeta;
40+
use crate::tenant_key::resource::TenantResource;
41+
42+
pub struct Resource;
43+
impl TenantResource for Resource {
44+
const PREFIX: &'static str = "__fd_catalog_by_id";
45+
const TYPE: &'static str = "CatalogIdIdent";
46+
const HAS_TENANT: bool = false;
47+
type ValueType = CatalogMeta;
48+
}
49+
50+
impl kvapi::Value for CatalogMeta {
51+
fn dependency_keys(&self) -> impl IntoIterator<Item = String> {
52+
[]
53+
}
54+
}
55+
56+
// // Use these error types to replace usage of ErrorCode if possible.
57+
// impl From<ExistError<Resource>> for ErrorCode {
58+
// impl From<UnknownError<Resource>> for ErrorCode {
59+
}
60+
61+
#[cfg(test)]
62+
mod tests {
63+
use databend_common_meta_kvapi::kvapi::Key;
64+
65+
use super::CatalogIdIdent;
66+
use crate::tenant::Tenant;
67+
68+
#[test]
69+
fn test_background_job_id_ident() {
70+
let tenant = Tenant::new_literal("dummy");
71+
let ident = CatalogIdIdent::new(tenant, 3);
72+
73+
let key = ident.to_string_key();
74+
assert_eq!(key, "__fd_catalog_by_id/3");
75+
76+
assert_eq!(ident, CatalogIdIdent::from_str_key(&key).unwrap());
77+
}
78+
79+
#[test]
80+
fn test_background_job_id_ident_with_key_space() {
81+
// TODO(xp): implement this test
82+
// let tenant = Tenant::new_literal("test");
83+
// let ident = CatalogIdIdent::new(tenant, 3);
84+
//
85+
// let key = ident.to_string_key();
86+
// assert_eq!(key, "__fd_catalog_by_id/3");
87+
//
88+
// assert_eq!(ident, CatalogIdIdent::from_str_key(&key).unwrap());
89+
}
90+
}

src/meta/app/src/schema/catalog_name_ident.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,18 @@ mod kvapi_impl {
2828
use databend_common_meta_kvapi::kvapi;
2929
use databend_common_meta_kvapi::kvapi::Key;
3030

31-
use crate::schema::CatalogId;
31+
use crate::schema::CatalogIdIdent;
3232
use crate::tenant_key::resource::TenantResource;
3333

3434
pub struct Resource;
3535
impl TenantResource for Resource {
3636
const PREFIX: &'static str = "__fd_catalog";
3737
const TYPE: &'static str = "CatalogNameIdent";
3838
const HAS_TENANT: bool = true;
39-
type ValueType = CatalogId;
39+
type ValueType = CatalogIdIdent;
4040
}
4141

42-
impl kvapi::Value for CatalogId {
42+
impl kvapi::Value for CatalogIdIdent {
4343
fn dependency_keys(&self) -> impl IntoIterator<Item = String> {
4444
[self.to_string_key()]
4545
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
//! Schema types
1616
1717
pub mod catalog;
18+
pub mod catalog_id;
1819
pub mod catalog_name_ident;
1920
pub mod database_id_history_ident;
2021
pub mod database_name_ident;
@@ -33,6 +34,7 @@ mod table;
3334
mod virtual_column;
3435

3536
pub use catalog::*;
37+
pub use catalog_id::CatalogIdIdent;
3638
pub use catalog_name_ident::CatalogNameIdent;
3739
pub use create_option::CreateOption;
3840
pub use database::CreateDatabaseReply;

src/query/catalog/src/catalog/manager.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use databend_common_config::InnerConfig;
2222
use databend_common_exception::ErrorCode;
2323
use databend_common_exception::Result;
2424
use databend_common_meta_api::SchemaApi;
25-
use databend_common_meta_app::schema::CatalogId;
25+
use databend_common_meta_app::schema::CatalogIdIdent;
2626
use databend_common_meta_app::schema::CatalogInfo;
2727
use databend_common_meta_app::schema::CatalogMeta;
2828
use databend_common_meta_app::schema::CatalogNameIdent;
@@ -100,7 +100,7 @@ impl CatalogManager {
100100
ErrorCode::BadArguments(format!("unknown catalog type: {:?}", CatalogType::Hive))
101101
})?;
102102
let ctl = creator.try_create(&CatalogInfo {
103-
id: CatalogId { catalog_id: 0 }.into(),
103+
id: CatalogIdIdent::new(&tenant, 0).into(),
104104
name_ident: CatalogNameIdent::new(tenant.clone(), name).into(),
105105
meta: CatalogMeta {
106106
catalog_option: CatalogOption::Hive(HiveCatalogOption {

src/query/service/src/interpreters/interpreter_catalog_create.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,12 @@ use databend_common_catalog::catalog::CatalogManager;
1818
use databend_common_config::GlobalConfig;
1919
use databend_common_exception::ErrorCode;
2020
use databend_common_exception::Result;
21-
use databend_common_meta_app::schema::CatalogId;
21+
use databend_common_meta_app::schema::CatalogIdIdent;
2222
use databend_common_meta_app::schema::CatalogInfo;
2323
use databend_common_meta_app::schema::CatalogMeta;
2424
use databend_common_meta_app::schema::CatalogNameIdent;
2525
use databend_common_meta_app::schema::CatalogOption;
26+
use databend_common_meta_app::tenant::Tenant;
2627
use databend_common_sql::plans::CreateCatalogPlan;
2728
use databend_common_storages_fuse::TableContext;
2829
use log::debug;
@@ -72,7 +73,7 @@ impl Interpreter for CreateCatalogInterpreter {
7273
let ctl = catalog_manager
7374
.build_catalog(
7475
&CatalogInfo {
75-
id: CatalogId::default().into(),
76+
id: CatalogIdIdent::new(Tenant::new_literal("dummy"), 0).into(),
7677
name_ident: CatalogNameIdent::new(self.plan.tenant.clone(), &self.plan.catalog)
7778
.into(),
7879
meta: CatalogMeta {

0 commit comments

Comments
 (0)