NoahKusaba opened a new pull request, #80: URL: https://github.com/apache/datafusion-iceberg/pull/80
## Which issue does this PR close? - Supersedes #48, addressing https://github.com/apache/datafusion-iceberg/pull/48#discussion_r4245361898. ## Rationale for this change A codec decodes a table provider synchronously, so it can't call the async `IcebergCatalogTableProvider::try_new`. #48 added `new_with_schema(catalog, ident, schema)` for this, but it accepts any schema, so a provider can be built whose schema has nothing to do with its table. This PR gives codecs a sync constructor whose schema can only come from the table itself. ## What changes are included in this PR? - Adds `IcebergCatalogTableProvider::try_new_from_table(catalog, &table)`. It derives the schema from `table`'s current schema and doesn't contact the catalog. `try_new` now loads the table and delegates to it. ```rust let provider = IcebergCatalogTableProvider::try_new_from_table(catalog, &table)?; ``` - Adds `metadata_location()`: the metadata file of the table version the provider's schema comes from. A distributed engine sends it with the plan so that the remote side can rebuild the provider from the same table version, and so with the same schema, even if the table has changed since. - Factors the three identical catalog loads in `scan`, `insert_into` and `metadata_table` into one private `load_table`. The provider still keeps only the identifier, the Arrow schema and now the metadata location, and scans and inserts still load the table from the catalog each time. This only addresses the schema half of the review comment; the catalog still has to be passed as a live `Arc<dyn Catalog>`. ## Are there any user-facing changes? Two additions: `try_new_from_table` and `metadata_location()`. Nothing is removed or changed. One subtle difference: `try_new` now keeps the identifier of the table the catalog returns rather than the one it was asked for. These are the same for the catalogs in this repo, but a catalog that normalizes names would now have its normalized form used for later loads and commits. ## Are these changes tested? Yes. - `test_plan_nodes_are_inspectable` rebuilds the provider with `try_new_from_table` and checks its identifier, schema and metadata location. - `test_scan_after_schema_evolution_reads_provider_columns` rebuilds a provider from the table as it was before a column was added, and checks that it declares and scans the same columns as the original. -- 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]
