sdd commented on code in PR #1227:
URL: https://github.com/apache/iceberg-rust/pull/1227#discussion_r2054607716
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -656,7 +658,29 @@ impl ManifestFile {
/// Load [`Manifest`].
///
/// This method will also initialize inherited values of
[`ManifestEntry`], such as `sequence_number`.
- pub async fn load_manifest(&self, file_io: &FileIO) -> Result<Manifest> {
+ ///
+ /// If `object_cache` is provided, it will be used to cache the manifest.
+ pub async fn load_manifest(
Review Comment:
> I agree that we should have `with_file_io`/`with_object_cache` in
`CatalogBuilder`/`TableBuilder` interface. However, an extra `TableContext` in
public api seems a little odd to me. All these things, `FileIO`, `ObjectCache`
also exists in `Catalog`. WDYT the `ManifestLoader`/`ManifestListLoader`
solution?
That's fair, I'm happy to not have `TableContext` for now and to re-assess
if we discover we need to pass a third item around in the same way.
Re `ManifestLoader` / `ManifestListLoader`: I'm not 100% clear what your
vision is here. Are you thinking that, if a public consumer needed direct
access to manifests / manifest lists for a table, they'd do something like:
```rust
let manifest_list_loader: ManifestListLoader =
table_foo.manifest_list_loader();
let manifest_loader: ManifestLoader = table_foo.manifest_loader();
let manifest_list: ManifestList =
manifest_list_loader.load_for_current_snapshot()?;
// or load via snapshot ID or file URI?
let manifest: Manifest = manifest_loader.load("s3://blah")?;
// re-use manifest loader to load as many manifests as you like for table foo
```
If so, I think this seems reasonable. Are you thinking that the table scan
plan would itself use the ManifestLoader also, internally?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
