flyrain commented on code in PR #4091:
URL: https://github.com/apache/polaris/pull/4091#discussion_r3029332460
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -302,6 +291,23 @@ public ListNamespacesResponse listNamespaces(Namespace
parent) {
return catalogHandlerUtils().listNamespaces(namespaceCatalog, parent);
}
+ public ListNamespacesResponse listNamespaces(
+ Namespace parent, String pageToken, Integer pageSize) {
+ PolarisAuthorizableOperation op =
PolarisAuthorizableOperation.LIST_NAMESPACES;
+ authorizeBasicNamespaceOperationOrThrow(op, parent);
+
+ if (isFederated) {
+ return catalogHandlerUtils().listNamespaces(namespaceCatalog, parent,
pageToken, pageSize);
+ } else {
+ PageToken pageRequest = PageToken.build(pageToken, pageSize,
this::shouldDecodeToken);
+ var results = ((IcebergCatalog) baseCatalog).listNamespaces(parent,
pageRequest);
Review Comment:
I think `isFederated` is exactly the semantic we want to express here. It
captures the intent directly, rather than coupling the logic to a specific
catalog class.
The goal is to distinguish behavior based on whether a catalog is federated
or not, not based on its concrete type. Tying this to `instanceof
IcebergCatalog` would mix implementation detail with business logic, which
feels less clear to me.
Do you agree that modeling this as an explicit semantic, instead of
inferring it from class type, is the right direction?
--
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]
