JingsongLi commented on PR #733:
URL: https://github.com/apache/paimon-rust/pull/733#issuecomment-5370756038
[P2] Keep DataFusion engine registration out of the core Catalog contract
Could we avoid `Catalog::load_table_routing(..., engine_types)`?
`engine_types` is DataFusion/session state, not catalog metadata. Passing it
into `paimon::Catalog` makes loading the same table depend on which
integration/resolvers are currently registered, and it forces third-party
catalogs to implement a DataFusion-specific routing protocol (while the default
implementation ignores the set).
I do not think simply calling `get_table()` and then inspecting `TableType`
in DataFusion is sufficient: by then FS/REST has entered the concrete Paimon
`Table` construction path; REST may acquire a data token and construct FileIO,
and the external-type fail-closed guard must run before that construction.
Calling `get_table_type()` and then `get_table()` would also fetch metadata
twice for native tables.
Could core instead expose an unconditional classification, for example:
```rust
async fn load_table(&self, id: &Identifier) -> Result<LoadedTable>;
enum LoadedTable {
Paimon(Box<Table>),
External(ExternalTableMetadata),
}
```
The catalog would load metadata once and decide only whether the core reader
can construct the table. Object/Lance/Iceberg would always return `External`.
DataFusion would then look up its own resolver and report a clear error if none
is registered. The existing `get_table()` path can remain fail-closed for
external types. In other words, core owns classification and stored-metadata
validation; DataFusion owns resolver selection, session semantics, and the
read-only wrapper.
This would also remove a hidden invariant in the current public API:
`RoutedTableLoad::engine` does not check `declared.requires_table_engine()`. A
non-DataFusion caller can include a native type in `engine_types` and make the
FS/REST implementations skip native table construction. Correctness currently
relies on the private check in the DataFusion registration path.
This boundary also matches the Java direction more closely: core dispatches
from table metadata without accepting caller-registered engine types.
--
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]
