Skip to content

Commit 195fb73

Browse files
stevie9868stevie9868
authored
Refactor: use metadatalocaiton:new_with_table_location in catalog create_table (#1558)
1 parent 3ab45ee commit 195fb73

File tree

12 files changed

+27
-147
lines changed

12 files changed

+27
-147
lines changed

‎Cargo.lock

Lines changed: 0 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/catalog/glue/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ serde_json = { workspace = true }
3838
tokio = { workspace = true }
3939
tracing = { workspace = true }
4040
typed-builder = { workspace = true }
41-
uuid = { workspace = true }
4241

4342
[dev-dependencies]
4443
ctor = { workspace = true }

‎crates/catalog/glue/src/catalog.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,15 @@ use iceberg::io::{
2626
use iceberg::spec::{TableMetadata, TableMetadataBuilder};
2727
use iceberg::table::Table;
2828
use iceberg::{
29-
Catalog, Error, ErrorKind, Namespace, NamespaceIdent, Result, TableCommit, TableCreation,
30-
TableIdent,
29+
Catalog, Error, ErrorKind, MetadataLocation, Namespace, NamespaceIdent, Result, TableCommit,
30+
TableCreation, TableIdent,
3131
};
3232
use typed_builder::TypedBuilder;
3333

3434
use crate::error::{from_aws_build_error, from_aws_sdk_error};
3535
use crate::utils::{
36-
convert_to_database, convert_to_glue_table, convert_to_namespace, create_metadata_location,
37-
create_sdk_config, get_default_table_location, get_metadata_location, validate_namespace,
36+
convert_to_database, convert_to_glue_table, convert_to_namespace, create_sdk_config,
37+
get_default_table_location, get_metadata_location, validate_namespace,
3838
};
3939
use crate::{
4040
AWS_ACCESS_KEY_ID, AWS_REGION_NAME, AWS_SECRET_ACCESS_KEY, AWS_SESSION_TOKEN, with_catalog_id,
@@ -393,7 +393,8 @@ impl Catalog for GlueCatalog {
393393
let metadata = TableMetadataBuilder::from_table_creation(creation)?
394394
.build()?
395395
.metadata;
396-
let metadata_location = create_metadata_location(&location, 0)?;
396+
let metadata_location =
397+
MetadataLocation::new_with_table_location(location.clone()).to_string();
397398

398399
metadata.write_to(&self.file_io, &metadata_location).await?;
399400

‎crates/catalog/glue/src/utils.rs

Lines changed: 2 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ use aws_sdk_glue::config::Credentials;
2222
use aws_sdk_glue::types::{Database, DatabaseInput, StorageDescriptor, TableInput};
2323
use iceberg::spec::TableMetadata;
2424
use iceberg::{Error, ErrorKind, Namespace, NamespaceIdent, Result};
25-
use uuid::Uuid;
2625

2726
use crate::error::from_aws_build_error;
2827
use crate::schema::GlueSchemaBuilder;
@@ -228,30 +227,6 @@ pub(crate) fn get_default_table_location(
228227
}
229228
}
230229

231-
/// Create metadata location from `location` and `version`
232-
pub(crate) fn create_metadata_location(location: impl AsRef<str>, version: i32) -> Result<String> {
233-
if version < 0 {
234-
return Err(Error::new(
235-
ErrorKind::DataInvalid,
236-
format!(
237-
"Table metadata version: '{}' must be a non-negative integer",
238-
version
239-
),
240-
));
241-
};
242-
243-
let version = format!("{:0>5}", version);
244-
let id = Uuid::new_v4();
245-
let metadata_location = format!(
246-
"{}/metadata/{}-{}.metadata.json",
247-
location.as_ref(),
248-
version,
249-
id
250-
);
251-
252-
Ok(metadata_location)
253-
}
254-
255230
/// Get metadata location from `GlueTable` parameters
256231
pub(crate) fn get_metadata_location(
257232
parameters: &Option<HashMap<String, String>>,
@@ -288,7 +263,7 @@ mod tests {
288263
use aws_sdk_glue::config::ProvideCredentials;
289264
use aws_sdk_glue::types::Column;
290265
use iceberg::spec::{NestedField, PrimitiveType, Schema, TableMetadataBuilder, Type};
291-
use iceberg::{Namespace, Result, TableCreation};
266+
use iceberg::{MetadataLocation, Namespace, Result, TableCreation};
292267

293268
use super::*;
294269
use crate::schema::{ICEBERG_FIELD_CURRENT, ICEBERG_FIELD_ID, ICEBERG_FIELD_OPTIONAL};
@@ -332,7 +307,7 @@ mod tests {
332307
fn test_convert_to_glue_table() -> Result<()> {
333308
let table_name = "my_table".to_string();
334309
let location = "s3a://warehouse/hive".to_string();
335-
let metadata_location = create_metadata_location(location.clone(), 0)?;
310+
let metadata_location = MetadataLocation::new_with_table_location(location).to_string();
336311
let properties = HashMap::new();
337312
let schema = Schema::builder()
338313
.with_schema_id(1)
@@ -372,22 +347,6 @@ mod tests {
372347
Ok(())
373348
}
374349

375-
#[test]
376-
fn test_create_metadata_location() -> Result<()> {
377-
let location = "my_base_location";
378-
let valid_version = 0;
379-
let invalid_version = -1;
380-
381-
let valid_result = create_metadata_location(location, valid_version)?;
382-
let invalid_result = create_metadata_location(location, invalid_version);
383-
384-
assert!(valid_result.starts_with("my_base_location/metadata/00000-"));
385-
assert!(valid_result.ends_with(".metadata.json"));
386-
assert!(invalid_result.is_err());
387-
388-
Ok(())
389-
}
390-
391350
#[test]
392351
fn test_get_default_table_location() -> Result<()> {
393352
let properties = HashMap::from([(LOCATION.to_string(), "db_location".to_string())]);

‎crates/catalog/hms/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ serde_json = { workspace = true }
3939
tokio = { workspace = true }
4040
tracing = { workspace = true }
4141
typed-builder = { workspace = true }
42-
uuid = { workspace = true }
4342
volo-thrift = { workspace = true }
4443

4544
# Transitive dependencies below

‎crates/catalog/hms/src/catalog.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ use iceberg::io::FileIO;
2929
use iceberg::spec::{TableMetadata, TableMetadataBuilder};
3030
use iceberg::table::Table;
3131
use iceberg::{
32-
Catalog, Error, ErrorKind, Namespace, NamespaceIdent, Result, TableCommit, TableCreation,
33-
TableIdent,
32+
Catalog, Error, ErrorKind, MetadataLocation, Namespace, NamespaceIdent, Result, TableCommit,
33+
TableCreation, TableIdent,
3434
};
3535
use typed_builder::TypedBuilder;
3636
use volo_thrift::MaybeException;
@@ -351,7 +351,8 @@ impl Catalog for HmsCatalog {
351351
.build()?
352352
.metadata;
353353

354-
let metadata_location = create_metadata_location(&location, 0)?;
354+
let metadata_location =
355+
MetadataLocation::new_with_table_location(location.clone()).to_string();
355356

356357
metadata.write_to(&self.file_io, &metadata_location).await?;
357358

‎crates/catalog/hms/src/utils.rs

Lines changed: 3 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ use hive_metastore::{Database, PrincipalType, SerDeInfo, StorageDescriptor};
2222
use iceberg::spec::Schema;
2323
use iceberg::{Error, ErrorKind, Namespace, NamespaceIdent, Result};
2424
use pilota::{AHashMap, FastStr};
25-
use uuid::Uuid;
2625

2726
use crate::schema::HiveSchemaBuilder;
2827

@@ -249,30 +248,6 @@ pub(crate) fn get_default_table_location(
249248
format!("{}/{}", location, table_name.as_ref())
250249
}
251250

252-
/// Create metadata location from `location` and `version`
253-
pub(crate) fn create_metadata_location(location: impl AsRef<str>, version: i32) -> Result<String> {
254-
if version < 0 {
255-
return Err(Error::new(
256-
ErrorKind::DataInvalid,
257-
format!(
258-
"Table metadata version: '{}' must be a non-negative integer",
259-
version
260-
),
261-
));
262-
};
263-
264-
let version = format!("{:0>5}", version);
265-
let id = Uuid::new_v4();
266-
let metadata_location = format!(
267-
"{}/metadata/{}-{}.metadata.json",
268-
location.as_ref(),
269-
version,
270-
id
271-
);
272-
273-
Ok(metadata_location)
274-
}
275-
276251
/// Get metadata location from `HiveTable` parameters
277252
pub(crate) fn get_metadata_location(
278253
parameters: &Option<AHashMap<FastStr, FastStr>>,
@@ -339,7 +314,7 @@ fn get_current_time() -> Result<i32> {
339314
#[cfg(test)]
340315
mod tests {
341316
use iceberg::spec::{NestedField, PrimitiveType, Type};
342-
use iceberg::{Namespace, NamespaceIdent};
317+
use iceberg::{MetadataLocation, Namespace, NamespaceIdent};
343318

344319
use super::*;
345320

@@ -370,7 +345,8 @@ mod tests {
370345
let db_name = "my_db".to_string();
371346
let table_name = "my_table".to_string();
372347
let location = "s3a://warehouse/hms".to_string();
373-
let metadata_location = create_metadata_location(location.clone(), 0)?;
348+
let metadata_location =
349+
MetadataLocation::new_with_table_location(location.clone()).to_string();
374350
let properties = HashMap::new();
375351
let schema = Schema::builder()
376352
.with_schema_id(1)
@@ -414,22 +390,6 @@ mod tests {
414390
Ok(())
415391
}
416392

417-
#[test]
418-
fn test_create_metadata_location() -> Result<()> {
419-
let location = "my_base_location";
420-
let valid_version = 0;
421-
let invalid_version = -1;
422-
423-
let valid_result = create_metadata_location(location, valid_version)?;
424-
let invalid_result = create_metadata_location(location, invalid_version);
425-
426-
assert!(valid_result.starts_with("my_base_location/metadata/00000-"));
427-
assert!(valid_result.ends_with(".metadata.json"));
428-
assert!(invalid_result.is_err());
429-
430-
Ok(())
431-
}
432-
433393
#[test]
434394
fn test_get_default_table_location() -> Result<()> {
435395
let properties = HashMap::from([(LOCATION.to_string(), "db_location".to_string())]);

‎crates/catalog/s3tables/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ aws-config = { workspace = true }
3535
aws-sdk-s3tables = "1.10.0"
3636
iceberg = { workspace = true }
3737
typed-builder = { workspace = true }
38-
uuid = { workspace = true, features = ["v4"] }
3938

4039
[dev-dependencies]
4140
iceberg_test_utils = { path = "../../test_utils", features = ["tests"] }

‎crates/catalog/s3tables/src/catalog.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ use iceberg::io::{FileIO, FileIOBuilder};
2828
use iceberg::spec::{TableMetadata, TableMetadataBuilder};
2929
use iceberg::table::Table;
3030
use iceberg::{
31-
Catalog, Error, ErrorKind, Namespace, NamespaceIdent, Result, TableCommit, TableCreation,
32-
TableIdent,
31+
Catalog, Error, ErrorKind, MetadataLocation, Namespace, NamespaceIdent, Result, TableCommit,
32+
TableCreation, TableIdent,
3333
};
3434
use typed_builder::TypedBuilder;
3535

36-
use crate::utils::{create_metadata_location, create_sdk_config};
36+
use crate::utils::create_sdk_config;
3737

3838
/// S3Tables catalog configuration.
3939
#[derive(Debug, TypedBuilder)]
@@ -325,7 +325,7 @@ impl Catalog for S3TablesCatalog {
325325
.await
326326
.map_err(from_aws_sdk_error)?;
327327
let warehouse_location = get_resp.warehouse_location().to_string();
328-
create_metadata_location(warehouse_location, 0)?
328+
MetadataLocation::new_with_table_location(warehouse_location).to_string()
329329
}
330330
};
331331

‎crates/catalog/s3tables/src/utils.rs

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@ use std::collections::HashMap;
1919

2020
use aws_config::{BehaviorVersion, Region, SdkConfig};
2121
use aws_sdk_s3tables::config::Credentials;
22-
use iceberg::{Error, ErrorKind, Result};
23-
use uuid::Uuid;
2422

2523
/// Property aws profile name
2624
pub const AWS_PROFILE_NAME: &str = "profile_name";
@@ -71,30 +69,3 @@ pub(crate) async fn create_sdk_config(
7169

7270
config.load().await
7371
}
74-
75-
/// Create metadata location from `location` and `version`
76-
pub(crate) fn create_metadata_location(
77-
warehouse_location: impl AsRef<str>,
78-
version: i32,
79-
) -> Result<String> {
80-
if version < 0 {
81-
return Err(Error::new(
82-
ErrorKind::DataInvalid,
83-
format!(
84-
"Table metadata version: '{}' must be a non-negative integer",
85-
version
86-
),
87-
));
88-
};
89-
90-
let version = format!("{:0>5}", version);
91-
let id = Uuid::new_v4();
92-
let metadata_location = format!(
93-
"{}/metadata/{}-{}.metadata.json",
94-
warehouse_location.as_ref(),
95-
version,
96-
id
97-
);
98-
99-
Ok(metadata_location)
100-
}

0 commit comments

Comments
 (0)