JingsongLi commented on code in PR #733:
URL: https://github.com/apache/paimon-rust/pull/733#discussion_r3834864856
##########
crates/paimon/src/catalog/mod.rs:
##########
@@ -323,6 +371,20 @@ pub trait Catalog: Send + Sync {
/// * [`crate::Error::TableNotExist`] - table does not exist.
async fn get_table(&self, identifier: &Identifier) -> Result<Table>;
+ /// Load a table, or classify it as [`LoadedTable::External`] when this
+ /// reader cannot construct it. One metadata round-trip either way, and the
+ /// outcome depends only on the table's own metadata. The default
+ /// implementation always constructs, for catalogs without a table-type
+ /// concept.
+ ///
+ /// # Errors
+ /// Same as [`Catalog::get_table`].
+ async fn load_table(&self, identifier: &Identifier) -> Result<LoadedTable>
{
Review Comment:
[P1] Classify the table in the default `load_table` implementation
This default is the compatibility path for existing third-party `Catalog`
implementations, but it unconditionally wraps the `Table` returned by
`get_table` as `LoadedTable::Paimon`. If such a catalog returns a table whose
stored `type` is `object-table`, `lance-table`, or `iceberg-table`,
`PaimonSchemaProvider::table` takes the Paimon arm and never consults the
registered resolver. `PaimonTableProvider` has no later type guard, so reads
can misread the foreign table (or silently appear empty), and INSERT can write
Paimon metadata into the foreign location.
This also contradicts the PR description that the default "classifies from
the constructed table" so a catalog implementing only `get_table` still fails
closed. Please first load the table, parse
`CoreOptions::new(table.schema().options()).table_type()?`, and call the
checked `LoadedTable::external` constructor when `requires_table_engine()` is
true; otherwise return the Paimon variant. The built-in overrides can remain to
preserve pre-construction classification and avoid token/FileIO work. A
regression test should use a catalog that deliberately does not override
`load_table`.
--
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]
