Skip to content

Commit 4f5d8e2

Browse files
authored
Rest: Implement register table (#1521)
## Which issue does this PR close? - Closes #1516. ## What changes are included in this PR? - New type for RegisterTableRequest - New method in rest catalog for implementing register table ## Are these changes tested? - 2 unit tests for a successful registered table operation and a failed one due to namespace is non existent - Integration tests for a successful registered table operation Im new to Rust so any feedback is welcomed! :)
1 parent c390ad3 commit 4f5d8e2

File tree

3 files changed

+186
-8
lines changed

3 files changed

+186
-8
lines changed

crates/catalog/rest/src/catalog.rs

Lines changed: 142 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ use crate::client::{
4242
use crate::types::{
4343
CatalogConfig, CommitTableRequest, CommitTableResponse, CreateTableRequest,
4444
ListNamespaceResponse, ListTableResponse, LoadTableResponse, NamespaceSerde,
45-
RenameTableRequest,
45+
RegisterTableRequest, RenameTableRequest,
4646
};
4747

4848
const ICEBERG_REST_SPEC_VERSION: &str = "0.14.1";
@@ -101,6 +101,10 @@ impl RestCatalogConfig {
101101
self.url_prefixed(&["tables", "rename"])
102102
}
103103

104+
fn register_table_endpoint(&self, ns: &NamespaceIdent) -> String {
105+
self.url_prefixed(&["namespaces", &ns.to_url_string(), "register"])
106+
}
107+
104108
fn table_endpoint(&self, table: &TableIdent) -> String {
105109
self.url_prefixed(&[
106110
"namespaces",
@@ -238,7 +242,7 @@ struct RestContext {
238242
pub struct RestCatalog {
239243
/// User config is stored as-is and never be changed.
240244
///
241-
/// It's could be different from the config fetched from the server and used at runtime.
245+
/// It could be different from the config fetched from the server and used at runtime.
242246
user_config: RestCatalogConfig,
243247
ctx: OnceCell<RestContext>,
244248
/// Extensions for the FileIOBuilder.
@@ -755,13 +759,60 @@ impl Catalog for RestCatalog {
755759

756760
async fn register_table(
757761
&self,
758-
_table_ident: &TableIdent,
759-
_metadata_location: String,
762+
table_ident: &TableIdent,
763+
metadata_location: String,
760764
) -> Result<Table> {
761-
Err(Error::new(
762-
ErrorKind::FeatureUnsupported,
763-
"Registering a table is not supported yet",
764-
))
765+
let context = self.context().await?;
766+
767+
let request = context
768+
.client
769+
.request(
770+
Method::POST,
771+
context
772+
.config
773+
.register_table_endpoint(table_ident.namespace()),
774+
)
775+
.json(&RegisterTableRequest {
776+
name: table_ident.name.clone(),
777+
metadata_location: metadata_location.clone(),
778+
overwrite: Some(false),
779+
})
780+
.build()?;
781+
782+
let http_response = context.client.query_catalog(request).await?;
783+
784+
let response: LoadTableResponse = match http_response.status() {
785+
StatusCode::OK => {
786+
deserialize_catalog_response::<LoadTableResponse>(http_response).await?
787+
}
788+
StatusCode::NOT_FOUND => {
789+
return Err(Error::new(
790+
ErrorKind::NamespaceNotFound,
791+
"The namespace specified does not exist.",
792+
));
793+
}
794+
StatusCode::CONFLICT => {
795+
return Err(Error::new(
796+
ErrorKind::TableAlreadyExists,
797+
"The given table already exists.",
798+
));
799+
}
800+
_ => return Err(deserialize_unexpected_catalog_error(http_response).await),
801+
};
802+
803+
let metadata_location = response.metadata_location.as_ref().ok_or(Error::new(
804+
ErrorKind::DataInvalid,
805+
"Metadata location missing in `register_table` response!",
806+
))?;
807+
808+
let file_io = self.load_file_io(Some(metadata_location), None).await?;
809+
810+
Table::builder()
811+
.identifier(table_ident.clone())
812+
.file_io(file_io)
813+
.metadata(response.metadata)
814+
.metadata_location(metadata_location.clone())
815+
.build()
765816
}
766817

767818
async fn update_table(&self, mut commit: TableCommit) -> Result<Table> {
@@ -2470,4 +2521,87 @@ mod tests {
24702521
update_table_mock.assert_async().await;
24712522
load_table_mock.assert_async().await;
24722523
}
2524+
2525+
#[tokio::test]
2526+
async fn test_register_table() {
2527+
let mut server = Server::new_async().await;
2528+
2529+
let config_mock = create_config_mock(&mut server).await;
2530+
2531+
let register_table_mock = server
2532+
.mock("POST", "/v1/namespaces/ns1/register")
2533+
.with_status(200)
2534+
.with_body_from_file(format!(
2535+
"{}/testdata/{}",
2536+
env!("CARGO_MANIFEST_DIR"),
2537+
"load_table_response.json"
2538+
))
2539+
.create_async()
2540+
.await;
2541+
2542+
let catalog = RestCatalog::new(RestCatalogConfig::builder().uri(server.url()).build());
2543+
let table_ident =
2544+
TableIdent::new(NamespaceIdent::new("ns1".to_string()), "test1".to_string());
2545+
let metadata_location = String::from(
2546+
"s3://warehouse/database/table/metadata/00001-5f2f8166-244c-4eae-ac36-384ecdec81fc.gz.metadata.json",
2547+
);
2548+
2549+
let table = catalog
2550+
.register_table(&table_ident, metadata_location)
2551+
.await
2552+
.unwrap();
2553+
2554+
assert_eq!(
2555+
&TableIdent::from_strs(vec!["ns1", "test1"]).unwrap(),
2556+
table.identifier()
2557+
);
2558+
assert_eq!(
2559+
"s3://warehouse/database/table/metadata/00001-5f2f8166-244c-4eae-ac36-384ecdec81fc.gz.metadata.json",
2560+
table.metadata_location().unwrap()
2561+
);
2562+
2563+
config_mock.assert_async().await;
2564+
register_table_mock.assert_async().await;
2565+
}
2566+
2567+
#[tokio::test]
2568+
async fn test_register_table_404() {
2569+
let mut server = Server::new_async().await;
2570+
2571+
let config_mock = create_config_mock(&mut server).await;
2572+
2573+
let register_table_mock = server
2574+
.mock("POST", "/v1/namespaces/ns1/register")
2575+
.with_status(404)
2576+
.with_body(
2577+
r#"
2578+
{
2579+
"error": {
2580+
"message": "The namespace specified does not exist",
2581+
"type": "NoSuchNamespaceErrorException",
2582+
"code": 404
2583+
}
2584+
}
2585+
"#,
2586+
)
2587+
.create_async()
2588+
.await;
2589+
2590+
let catalog = RestCatalog::new(RestCatalogConfig::builder().uri(server.url()).build());
2591+
2592+
let table_ident =
2593+
TableIdent::new(NamespaceIdent::new("ns1".to_string()), "test1".to_string());
2594+
let metadata_location = String::from(
2595+
"s3://warehouse/database/table/metadata/00001-5f2f8166-244c-4eae-ac36-384ecdec81fc.gz.metadata.json",
2596+
);
2597+
let table = catalog
2598+
.register_table(&table_ident, metadata_location)
2599+
.await;
2600+
2601+
assert!(table.is_err());
2602+
assert!(table.err().unwrap().message().contains("does not exist"));
2603+
2604+
config_mock.assert_async().await;
2605+
register_table_mock.assert_async().await;
2606+
}
24732607
}

crates/catalog/rest/src/types.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,3 +191,11 @@ pub(super) struct CommitTableResponse {
191191
pub(super) metadata_location: String,
192192
pub(super) metadata: TableMetadata,
193193
}
194+
195+
#[derive(Debug, Serialize, Deserialize)]
196+
#[serde(rename_all = "kebab-case")]
197+
pub(super) struct RegisterTableRequest {
198+
pub(super) name: String,
199+
pub(super) metadata_location: String,
200+
pub(super) overwrite: Option<bool>,
201+
}

crates/catalog/rest/tests/rest_catalog_test.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,3 +407,39 @@ async fn test_list_empty_multi_level_namespace() {
407407
.unwrap();
408408
assert!(nss.is_empty());
409409
}
410+
411+
#[tokio::test]
412+
async fn test_register_table() {
413+
let catalog = get_catalog().await;
414+
415+
// Create namespace
416+
let ns = NamespaceIdent::from_strs(["ns"]).unwrap();
417+
catalog.create_namespace(&ns, HashMap::new()).await.unwrap();
418+
419+
// Create the table, store the metadata location, drop the table
420+
let empty_schema = Schema::builder().build().unwrap();
421+
let table_creation = TableCreation::builder()
422+
.name("t1".to_string())
423+
.schema(empty_schema)
424+
.build();
425+
426+
let table = catalog.create_table(&ns, table_creation).await.unwrap();
427+
428+
let metadata_location = table.metadata_location().unwrap();
429+
catalog.drop_table(table.identifier()).await.unwrap();
430+
431+
let new_table_identifier = TableIdent::from_strs(["ns", "t2"]).unwrap();
432+
let table_registered = catalog
433+
.register_table(&new_table_identifier, metadata_location.to_string())
434+
.await
435+
.unwrap();
436+
437+
assert_eq!(
438+
table.metadata_location(),
439+
table_registered.metadata_location()
440+
);
441+
assert_ne!(
442+
table.identifier().to_string(),
443+
table_registered.identifier().to_string()
444+
);
445+
}

0 commit comments

Comments
 (0)