Skip to content

Commit a1dcb4a

Browse files
fqaiser94fqaiser94
committed
Minor clean up
1 parent cedf7cc commit a1dcb4a

File tree

2 files changed

+81
-95
lines changed

2 files changed

+81
-95
lines changed

‎crates/catalog/inmemory/src/catalog.rs‎

Lines changed: 56 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -53,30 +53,6 @@ impl InMemoryCatalog {
5353
}
5454
}
5555

56-
/// Create metadata location from `location` and `version`
57-
fn create_metadata_location(location: impl AsRef<str>, version: i32) -> Result<String> {
58-
if version < 0 {
59-
Err(Error::new(
60-
ErrorKind::DataInvalid,
61-
format!(
62-
"Table metadata version: '{}' must be a non-negative integer",
63-
version
64-
),
65-
))
66-
} else {
67-
let version = format!("{:0>5}", version);
68-
let id = Uuid::new_v4();
69-
let metadata_location = format!(
70-
"{}/metadata/{}-{}.metadata.json",
71-
location.as_ref(),
72-
version,
73-
id
74-
);
75-
76-
Ok(metadata_location)
77-
}
78-
}
79-
8056
#[async_trait]
8157
impl Catalog for InMemoryCatalog {
8258
/// List namespaces inside the Catalog.
@@ -87,11 +63,15 @@ impl Catalog for InMemoryCatalog {
8763
let root_namespace_state = self.root_namespace_state.lock().await;
8864

8965
match maybe_parent {
90-
None => Ok(root_namespace_state
91-
.list_top_level_namespaces()
92-
.into_iter()
93-
.map(|str| NamespaceIdent::new(str.to_string()))
94-
.collect_vec()),
66+
None => {
67+
let namespaces = root_namespace_state
68+
.list_top_level_namespaces()
69+
.into_iter()
70+
.map(|str| NamespaceIdent::new(str.to_string()))
71+
.collect_vec();
72+
73+
Ok(namespaces)
74+
}
9575
Some(parent_namespace_ident) => {
9676
let namespaces = root_namespace_state
9777
.list_namespaces_under(parent_namespace_ident)?
@@ -184,55 +164,49 @@ impl Catalog for InMemoryCatalog {
184164

185165
let table_name = table_creation.name.clone();
186166
let table_ident = TableIdent::new(namespace_ident.clone(), table_name);
187-
let table_exists = root_namespace_state.table_exists(&table_ident)?;
188-
189-
if table_exists {
190-
Err(Error::new(
191-
ErrorKind::Unexpected,
192-
format!(
193-
"Cannot create table {:?}. Table already exists",
194-
&table_ident
195-
),
196-
))
197-
} else {
198-
let (table_creation, location) = match table_creation.location.clone() {
199-
Some(location) => (table_creation, location),
200-
None => {
201-
let location = format!(
202-
"{}/{}/{}",
203-
self.default_table_root_location,
204-
table_ident.namespace().join("/"),
205-
table_ident.name()
206-
);
207-
208-
let new_table_creation = TableCreation {
209-
location: Some(location.clone()),
210-
..table_creation
211-
};
212-
213-
(new_table_creation, location)
214-
}
215-
};
216-
217-
let metadata = TableMetadataBuilder::from_table_creation(table_creation)?.build()?;
218-
let metadata_location = create_metadata_location(&location, 0)?;
219-
220-
self.file_io
221-
.new_output(&metadata_location)?
222-
.write(serde_json::to_vec(&metadata)?.into())
223-
.await?;
224-
225-
root_namespace_state.insert_new_table(&table_ident, metadata_location.clone())?;
226-
227-
let table = Table::builder()
228-
.file_io(self.file_io.clone())
229-
.metadata_location(metadata_location)
230-
.metadata(metadata)
231-
.identifier(table_ident)
232-
.build();
233-
234-
Ok(table)
235-
}
167+
168+
let (table_creation, location) = match table_creation.location.clone() {
169+
Some(location) => (table_creation, location),
170+
None => {
171+
let location = format!(
172+
"{}/{}/{}",
173+
self.default_table_root_location,
174+
table_ident.namespace().join("/"),
175+
table_ident.name()
176+
);
177+
178+
let new_table_creation = TableCreation {
179+
location: Some(location.clone()),
180+
..table_creation
181+
};
182+
183+
(new_table_creation, location)
184+
}
185+
};
186+
187+
let metadata = TableMetadataBuilder::from_table_creation(table_creation)?.build()?;
188+
let metadata_location = format!(
189+
"{}/metadata/{}-{}.metadata.json",
190+
&location,
191+
0,
192+
Uuid::new_v4()
193+
);
194+
195+
self.file_io
196+
.new_output(&metadata_location)?
197+
.write(serde_json::to_vec(&metadata)?.into())
198+
.await?;
199+
200+
root_namespace_state.insert_new_table(&table_ident, metadata_location.clone())?;
201+
202+
let table = Table::builder()
203+
.file_io(self.file_io.clone())
204+
.metadata_location(metadata_location)
205+
.metadata(metadata)
206+
.identifier(table_ident)
207+
.build();
208+
209+
Ok(table)
236210
}
237211

238212
/// Load table from the catalog.
@@ -282,6 +256,7 @@ impl Catalog for InMemoryCatalog {
282256
new_root_namespace_state.remove_existing_table(src_table_ident)?;
283257
new_root_namespace_state.insert_new_table(dst_table_ident, metadata_location)?;
284258
*root_namespace_state = new_root_namespace_state;
259+
285260
Ok(())
286261
}
287262

@@ -604,7 +579,7 @@ mod tests {
604579
.unwrap_err()
605580
.to_string(),
606581
format!(
607-
"Unexpected => Cannot create namespace {:?}. Namespace already exists",
582+
"Unexpected => Cannot create namespace {:?}. Namespace already exists.",
608583
&namespace_ident
609584
)
610585
);
@@ -1092,7 +1067,7 @@ mod tests {
10921067
.unwrap_err()
10931068
.to_string(),
10941069
format!(
1095-
"Unexpected => Cannot create table {:?}. Table already exists",
1070+
"Unexpected => Cannot create table {:?}. Table already exists.",
10961071
&table_ident
10971072
)
10981073
);
@@ -1513,7 +1488,7 @@ mod tests {
15131488
.unwrap_err()
15141489
.to_string(),
15151490
format!(
1516-
"Unexpected => Cannot insert table {:? }. Table already exists.",
1491+
"Unexpected => Cannot create table {:? }. Table already exists.",
15171492
&dst_table_ident
15181493
),
15191494
);

‎crates/catalog/inmemory/src/namespace_state.rs‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,26 @@ fn no_such_table_err<T>(table_ident: &TableIdent) -> Result<T> {
4444
))
4545
}
4646

47+
fn namespace_already_exists_err<T>(namespace_ident: &NamespaceIdent) -> Result<T> {
48+
Err(Error::new(
49+
ErrorKind::Unexpected,
50+
format!(
51+
"Cannot create namespace {:?}. Namespace already exists.",
52+
namespace_ident
53+
),
54+
))
55+
}
56+
57+
fn table_already_exists_err<T>(table_ident: &TableIdent) -> Result<T> {
58+
Err(Error::new(
59+
ErrorKind::Unexpected,
60+
format!(
61+
"Cannot create table {:?}. Table already exists.",
62+
table_ident
63+
),
64+
))
65+
}
66+
4767
impl NamespaceState {
4868
// Creates a new namespace state
4969
pub(crate) fn new() -> Self {
@@ -157,19 +177,14 @@ impl NamespaceState {
157177
.namespaces
158178
.entry(child_namespace_name)
159179
{
160-
hash_map::Entry::Occupied(_) => Err(Error::new(
161-
ErrorKind::Unexpected,
162-
format!(
163-
"Cannot create namespace {:?}. Namespace already exists",
164-
namespace_ident
165-
),
166-
)),
180+
hash_map::Entry::Occupied(_) => namespace_already_exists_err(namespace_ident),
167181
hash_map::Entry::Vacant(entry) => {
168182
let _ = entry.insert(NamespaceState {
169183
properties,
170184
namespaces: HashMap::new(),
171185
table_metadata_locations: HashMap::new(),
172186
});
187+
173188
Ok(())
174189
}
175190
}
@@ -220,6 +235,7 @@ impl NamespaceState {
220235
) -> Result<()> {
221236
let properties = self.get_mut_properties(namespace_ident)?;
222237
*properties = new_properties;
238+
223239
Ok(())
224240
}
225241

@@ -266,15 +282,10 @@ impl NamespaceState {
266282
.table_metadata_locations
267283
.entry(table_ident.name().to_string())
268284
{
269-
hash_map::Entry::Occupied(_) => Err(Error::new(
270-
ErrorKind::Unexpected,
271-
format!(
272-
"Cannot insert table {:?}. Table already exists.",
273-
table_ident
274-
),
275-
)),
285+
hash_map::Entry::Occupied(_) => table_already_exists_err(table_ident),
276286
hash_map::Entry::Vacant(entry) => {
277287
let _ = entry.insert(metadata_location);
288+
278289
Ok(())
279290
}
280291
}

0 commit comments

Comments
 (0)