JingsongLi commented on code in PR #922:
URL: https://github.com/apache/paimon-rust/pull/922#discussion_r4091600069
##########
bindings/python/src/table.rs:
##########
@@ -83,6 +84,34 @@ impl PyTable {
Ok(Self::new(Arc::new(table)))
}
+ /// Reuse the matching REST table response and merged catalog options.
+ /// Skips config/get-table requests, preserving REST snapshots and token
refresh.
+ #[staticmethod]
+ #[pyo3(signature = (response_json, *, database, table, rest_options))]
Review Comment:
[P2] The dependent apache/paimon#10123 still calls
`Table.from_rest_response(..., options=_catalog_options(table))`. This binding
now accepts only `rest_options=`, so the real extension raises `TypeError`
before REST native planning. The dependent PR uses a mock and does not catch
keyword validation. Please update that caller and its test to `rest_options=`,
or preserve an `options` alias, and exercise the real binding.
##########
crates/paimon/src/table/rest_env.rs:
##########
@@ -336,6 +383,33 @@ impl RESTEnv {
}
}
+fn response_identifier(
+ requested: &Identifier,
+ response: &crate::api::GetTableResponse,
+) -> Result<Identifier> {
+ let database = response
+ .database
+ .as_deref()
+ .unwrap_or_else(|| requested.database());
+ let name = response.name.as_deref().ok_or_else(|| Error::DataInvalid {
+ message: format!("Table response for database '{database}' missing
name"),
+ source: None,
+ })?;
+ let identifier = Identifier::new(database, name);
+ identifier.validate()?;
+ if &identifier != requested {
Review Comment:
[P2] This exact-name check breaks the PyPaimon REST test server's branch
route: it strips `$branch_b1` before constructing the GET response, so
requesting `db.t$branch_b1` returns `name: "t"` and `RESTCatalog.get_table` now
raises `DataInvalid`. This is limited to that server; the Java and Rust servers
return the full object name. Please align the branch response contract or add a
safe compatibility path and cover it with a cross-client test. The linked
apache/paimon#10123 also passes the base name via `get_table_name()`; for
full-name responses it needs `get_object_name()`.
--
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]
