iceberg-rust icon indicating copy to clipboard operation
iceberg-rust copied to clipboard

api: Metastore Catalog API design

Open JanKaul opened this issue 2 years ago • 2 comments

Generally Iceberg catalogs function in two different ways. The rest catalog handles the updates to the table metadata internally. All other catalogs store the metadata in a metadata.json file and store the pointer to the current metadata file.

I would like to define a trait MetastoreCatalog(name is not important) that works only with the location of the metadata.json file. Additionally I would add an implementation for Catalog as follows:

trait MetastoreCatalog {
...
}

impl<T> Catalog for T
where
    T: MetastoreCatalog 
{
...
}

This way the non-rest catalogs only have to implement the metadata file logic through the MetastoreCatalog trait and will automatically implement Catalog. The goal is to reduce the redundancy between the catalog implementations.

The trait is going to look similar to the Catalog trait, the biggest difference will be in the update_table method which will look similar to this:

async fn update_table(&self, table: &TableIdent, metadata_location: &str, previous_metadata_location: &str) -> Result<String>;

If you agree with this approach I will work on a PR to add the trait.

JanKaul avatar Sep 21 '23 14:09 JanKaul

Hi, thanks for the suggestion.

Personally, I think we should prioritize adding some implementations first and then figure out how to merge duplicated code. For example, we can add storage catalog and hive catalog first to verify our design.

What are your thoughts?

Xuanwo avatar Sep 22 '23 02:09 Xuanwo

I didn't implement other catalogs before, so it's hard for me to predict if this abstraction is correct. Since this is just to avoid code duplication, I also agree with @Xuanwo that we should do concrete implementation first, and do the refactoring later when necessary without risking any breaking changes.

liurenjie1024 avatar Sep 24 '23 13:09 liurenjie1024

Catalog API has been added.

Xuanwo avatar Jul 05 '24 15:07 Xuanwo