FANNG1 commented on PR #10671:
URL: https://github.com/apache/gravitino/pull/10671#issuecomment-4646346531

   I found two issues:
   
   1. Blocking: `IcebergNamespaceOperations#listNamespaces` now decodes the 
`parent` query parameter with `RESTUtil.decodeNamespace(parent)`, which drops 
Gravitino's custom namespace separator 
(`IcebergRESTUtils.NAMESPACE_SEPARATOR_URLENCODED_UTF_8`). Other 
namespace/table/view paths still decode with that separator, and existing 
nested namespace requests encode parents with it. This can break `GET 
/namespaces?parent=...` for nested namespaces by decoding the parent 
incorrectly. Please keep the existing overload:
   
   ```java
   RESTUtil.decodeNamespace(parent, 
IcebergRESTUtils.NAMESPACE_SEPARATOR_URLENCODED_UTF_8)
   ```
   
   2. Minor: the PR description says pagination is offset-based and 
`nextPageToken` is the next offset, but the implementation is 
cursor/keyset-style: results are sorted by name and `nextPageToken` is the last 
item name from the current page. Please update the PR description so the 
documented behavior matches the implementation.
   
   Other than these, LGTM.


-- 
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]

Reply via email to