Smith-Cruise opened a new pull request, #3207:
URL: https://github.com/apache/iceberg-rust/pull/3207
## Which issue does this PR close?
No open issue tracks this. It is the same change as #2123, which was
approved by @gabotechs and +1'd by @derrley and @toutane, but was closed by the
stale bot rather than on review. Credit for the original patch goes to @askalt.
It is also a concrete instance of the kind of blocker @alamb asked for in #1797
("Reduce the need for `iceberg-rust` forks").
## What changes are included in this PR?
Makes `IcebergTableProvider::try_new` and
`IcebergTableProvider::metadata_table` `pub` instead of `pub(crate)`, adds doc
comments, and regenerates `public-api.txt`.
The reason, stated as code facts rather than preference:
1. **The crate's own docs and error message point at a type that cannot be
constructed.** The module docs say `IcebergTableProvider` is the one to "use
for write operations", and `IcebergStaticTableProvider::insert_into` errors
with *"Use `IcebergTableProvider` with a catalog for write support"*. But
`IcebergTableProvider` has zero public constructors, so an external crate
cannot follow that advice.
2. **The only public path, `IcebergCatalogProvider`, is not usable by an
engine that has its own catalog layer.**
- `IcebergCatalogProvider::try_new` calls `list_namespaces`, then per
namespace `list_tables`, then `load_table` for **every** table (`catalog.rs` /
`schema.rs`) — O(tables) catalog round trips before the first query is planned.
- The result is cached with no invalidation; the source's own TODOs say
schemas and tables "might become stale".
- `list_namespaces` results are flattened per segment and rebuilt with
`NamespaceIdent::new`, so a multi-level namespace `a.b` becomes two unrelated
schemas.
- It takes over the whole `CatalogProvider`, which an engine serving
Iceberg alongside other table formats under one catalog cannot give up.
3. **The visibility is asymmetric.**
`IcebergStaticTableProvider::try_new_from_table` and
`try_new_from_table_snapshot` are both already `pub`. Only the write-capable
provider is sealed. After #1879 split the type in two, the `pub` constructors
went to the read-only half and the write-capable half was left with none.
4. **No new API surface.** The struct, its `TableProvider` impl, and
`IcebergMetadataTableProvider` are all already public; the signature mirrors
the already-public `IcebergCatalogProvider::try_new(Arc<dyn Catalog>)`. The
change adds 6 lines to `public-api.txt` and nothing else.
`metadata_table` is included because an engine constructing the provider
directly also needs `table$snapshots` and friends; leaving it `pub(crate)`
would make metadata tables reachable only through `IcebergCatalogProvider`,
reintroducing the same problem for a subset of queries.
### Concrete use case
We are adding Iceberg `INSERT INTO` to a DataFusion-based engine that serves
Iceberg, Delta, Paimon and Hive tables under a single catalog abstraction, with
lazy per-statement table resolution. Because `try_new` is `pub(crate)`, the
only way we found to reach the write-capable provider was to implement a
~150-line `iceberg::Catalog` shim that wraps our real catalog and reports
exactly one namespace holding one table, purely to stop
`IcebergCatalogProvider::try_new` from walking the entire catalog. With
`try_new` public that shim collapses into a single call.
## Are these changes tested?
This is a visibility change with no behavior change, so it adds no tests;
existing unit and integration tests cover the provider itself. Verified
locally: `cargo build`, `cargo clippy -p iceberg-datafusion --all-targets
--all-features -- -D warnings`, `cargo test -p iceberg-datafusion --lib` (89
passed), `RUSTDOCFLAGS="-D warnings" cargo doc`, and `make check-public-api`
across all 11 crates.
## AI Disclosure
Written with assistance from Claude Code. The author reviewed the change and
ran the verification above.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
