Skip to content

Commit eceaadd

Browse files
authored
Optimize PolicyCatalog.listPolicies (#2370)
this is a follow-up to #2290 the optimization is to use `listEntities` instead of `loadEntities` when there is `policyType` filter to apply
1 parent d326962 commit eceaadd

File tree

3 files changed

+35
-11
lines changed

3 files changed

+35
-11
lines changed

polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyEntity.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,8 @@ public PolicyType getPolicyType() {
5353

5454
@JsonIgnore
5555
public int getPolicyTypeCode() {
56-
Preconditions.checkArgument(
57-
getPropertiesAsMap().containsKey(POLICY_TYPE_CODE_KEY),
58-
"Invalid policy entity: policy type must exist");
5956
String policyTypeCode = getPropertiesAsMap().get(POLICY_TYPE_CODE_KEY);
57+
Preconditions.checkNotNull(policyTypeCode, "Invalid policy entity: policy type must exist");
6058
return Integer.parseInt(policyTypeCode);
6159
}
6260

runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
import com.google.common.base.Strings;
2626
import jakarta.annotation.Nonnull;
27+
import jakarta.annotation.Nullable;
2728
import java.util.ArrayList;
2829
import java.util.Arrays;
2930
import java.util.HashSet;
@@ -43,13 +44,16 @@
4344
import org.apache.polaris.core.context.CallContext;
4445
import org.apache.polaris.core.entity.CatalogEntity;
4546
import org.apache.polaris.core.entity.PolarisEntity;
47+
import org.apache.polaris.core.entity.PolarisEntityCore;
4648
import org.apache.polaris.core.entity.PolarisEntitySubType;
4749
import org.apache.polaris.core.entity.PolarisEntityType;
4850
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
4951
import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper;
5052
import org.apache.polaris.core.persistence.PolicyMappingAlreadyExistsException;
5153
import org.apache.polaris.core.persistence.dao.entity.EntityResult;
54+
import org.apache.polaris.core.persistence.dao.entity.ListEntitiesResult;
5255
import org.apache.polaris.core.persistence.dao.entity.LoadPolicyMappingsResult;
56+
import org.apache.polaris.core.persistence.pagination.PageToken;
5357
import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView;
5458
import org.apache.polaris.core.policy.PolicyEntity;
5559
import org.apache.polaris.core.policy.PolicyType;
@@ -71,8 +75,8 @@ public class PolicyCatalog {
7175
private final CallContext callContext;
7276
private final PolarisResolutionManifestCatalogView resolvedEntityView;
7377
private final CatalogEntity catalogEntity;
74-
private long catalogId = -1;
75-
private PolarisMetaStoreManager metaStoreManager;
78+
private final long catalogId;
79+
private final PolarisMetaStoreManager metaStoreManager;
7680

7781
public PolicyCatalog(
7882
PolarisMetaStoreManager metaStoreManager,
@@ -153,24 +157,46 @@ public Policy createPolicy(
153157
return constructPolicy(resultEntity);
154158
}
155159

156-
public List<PolicyIdentifier> listPolicies(Namespace namespace, PolicyType policyType) {
160+
public List<PolicyIdentifier> listPolicies(Namespace namespace, @Nullable PolicyType policyType) {
157161
PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getResolvedPath(namespace);
158162
if (resolvedEntities == null) {
159163
throw new IllegalStateException(
160164
String.format("Failed to fetch resolved namespace '%s'", namespace));
161165
}
162166

163-
List<PolarisEntity> catalogPath = resolvedEntities.getRawFullPath();
164-
// TODO: when the "policyType" filter is null we should only call "listEntities" instead
167+
List<PolarisEntityCore> catalogPath =
168+
PolarisEntity.toCoreList(resolvedEntities.getRawFullPath());
169+
if (policyType == null) {
170+
// without a policyType filter we can call listEntities to acquire the entity names
171+
ListEntitiesResult listEntitiesResult =
172+
metaStoreManager.listEntities(
173+
callContext.getPolarisCallContext(),
174+
catalogPath,
175+
PolarisEntityType.POLICY,
176+
PolarisEntitySubType.NULL_SUBTYPE,
177+
PageToken.readEverything());
178+
if (!listEntitiesResult.isSuccess()) {
179+
throw new IllegalStateException("Failed to list policies in namespace: " + namespace);
180+
}
181+
return listEntitiesResult.getEntities().stream()
182+
.map(
183+
entity ->
184+
PolicyIdentifier.builder()
185+
.setNamespace(namespace)
186+
.setName(entity.getName())
187+
.build())
188+
.toList();
189+
}
190+
// with a policyType filter we need to load the full PolicyEntity to apply the filter
165191
return metaStoreManager
166192
.loadEntitiesAll(
167193
callContext.getPolarisCallContext(),
168-
PolarisEntity.toCoreList(catalogPath),
194+
catalogPath,
169195
PolarisEntityType.POLICY,
170196
PolarisEntitySubType.NULL_SUBTYPE)
171197
.stream()
172198
.map(PolicyEntity::of)
173-
.filter(policyEntity -> policyType == null || policyEntity.getPolicyType() == policyType)
199+
.filter(policyEntity -> policyEntity.getPolicyType() == policyType)
174200
.map(
175201
entity ->
176202
PolicyIdentifier.builder()

runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalogHandler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ protected void initializeCatalog() {
8787
this.policyCatalog = new PolicyCatalog(metaStoreManager, callContext, this.resolutionManifest);
8888
}
8989

90-
public ListPoliciesResponse listPolicies(Namespace parent, PolicyType policyType) {
90+
public ListPoliciesResponse listPolicies(Namespace parent, @Nullable PolicyType policyType) {
9191
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_POLICY;
9292
authorizeBasicNamespaceOperationOrThrow(op, parent);
9393

0 commit comments

Comments
 (0)