dennishuo commented on code in PR #2290:
URL: https://github.com/apache/polaris/pull/2290#discussion_r2280127415
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java:
##########
@@ -162,34 +161,15 @@ public List<PolicyIdentifier> listPolicies(Namespace
namespace, PolicyType polic
}
List<PolarisEntity> catalogPath = resolvedEntities.getRawFullPath();
- List<PolicyEntity> policyEntities =
- metaStoreManager
- .listEntities(
- callContext.getPolarisCallContext(),
- PolarisEntity.toCoreList(catalogPath),
- PolarisEntityType.POLICY,
- PolarisEntitySubType.NULL_SUBTYPE,
- PageToken.readEverything())
- .getEntities()
- .stream()
- .map(
- polarisEntityActiveRecord ->
- PolicyEntity.of(
- metaStoreManager
- .loadEntity(
- callContext.getPolarisCallContext(),
- polarisEntityActiveRecord.getCatalogId(),
- polarisEntityActiveRecord.getId(),
- polarisEntityActiveRecord.getType())
- .getEntity()))
- .filter(
- policyEntity -> policyType == null ||
policyEntity.getPolicyType() == policyType)
- .toList();
-
- List<PolarisEntity.NameAndId> entities =
- policyEntities.stream().map(PolarisEntity::nameAndId).toList();
-
- return entities.stream()
+ return metaStoreManager
+ .loadEntitiesAll(
+ callContext.getPolarisCallContext(),
+ PolarisEntity.toCoreList(catalogPath),
+ PolarisEntityType.POLICY,
+ PolarisEntitySubType.NULL_SUBTYPE,
+ PolicyEntity::of)
+ .stream()
+ .filter(policyEntity -> policyType == null ||
policyEntity.getPolicyType() == policyType)
Review Comment:
> side note:
if the required policyType is null we could in theory use the optimized
listEntities call, as we only need the name of the entity to build the
PolicyIdentifier, but for filtering by policyType we need to load the full
entity.
This could be worth adding in a code comment here as a `// TODO` either as
an optional followup task or if nothing else, calling attention to the
different flavors of listing so that future code changes will put more thought
into the choice of listing methods.
Ideally any use of these open-ended filters will either be a short-term
crutch or a proven "small" use case where we think pushdown isn't worth the
complexity. Longer term we could define a more structured definition of
pushdown predicates that is still extensible but communicates filter semantics
down to the BasePersistence layer enough to work with different database-level
indexes.
--
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]
