Skip to content

Commit ee04df4

Browse files
XN137XN137
authored
Add entitySubType param to BasePersistence.listEntities (#2317)
`BasePersistence.listEntities` has 3 variants: ``` Page<EntityNameLookupRecord> listEntities(..., PageToken); Page<EntityNameLookupRecord> listEntities(..., Predicate<PolarisBaseEntity>, PageToken) <T> Page<T> listEntities(..., Predicate<PolarisBaseEntity>, Function<PolarisBaseEntity, T>, PageToken); ``` the 1st method exists to only return the subset of entity properties required to build an `EntityNameLookupRecord`. the 3rd method supports a predicate and transformer function on the underlying `PolarisBaseEntity`, which means it has to load all entity properties. the 2nd method is weird as it supports a full `Predicate<PolarisBaseEntity>`, which means it has to load all entity properties under the hood for filtering but then throws most of them away to return a `EntityNameLookupRecord`. this explains why the implementations of the 2nd method simply forward to the 3rd method usually. any performance benefits of returning a `EntityNameLookupRecord` are lost. as it turns out the 2nd method is only used, because methods 1 and 3 dont support passing a `PolarisEntitySubType` parameter to filter down the retrieved data. Note that the sub type property is available from both the `PolarisBaseEntity` as well as the `EntityNameLookupRecord`. By adding this parameter, the 2nd method can go away completely. we can even push down the sub type filtering into the queries of some of our persistence implementations. other existing implementations are free to decide whether they want to push it down as well or filter on the query results in memory. note that since we have no `TransactionalPersistence` implementation in the codebase that provides an optimized variant of method 1 we can have a default method in the interface that forwards to method 3.
1 parent cdb58e5 commit ee04df4

File tree

9 files changed

+73
-167
lines changed

9 files changed

+73
-167
lines changed

‎persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java‎

Lines changed: 3 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
package org.apache.polaris.extension.persistence.impl.eclipselink;
2020

2121
import com.google.common.annotations.VisibleForTesting;
22-
import com.google.common.base.Predicates;
2322
import jakarta.annotation.Nonnull;
2423
import jakarta.annotation.Nullable;
2524
import jakarta.persistence.EntityManager;
@@ -49,6 +48,7 @@
4948
import org.apache.polaris.core.entity.PolarisEntity;
5049
import org.apache.polaris.core.entity.PolarisEntityCore;
5150
import org.apache.polaris.core.entity.PolarisEntityId;
51+
import org.apache.polaris.core.entity.PolarisEntitySubType;
5252
import org.apache.polaris.core.entity.PolarisEntityType;
5353
import org.apache.polaris.core.entity.PolarisGrantRecord;
5454
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
@@ -424,58 +424,21 @@ public List<EntityNameLookupRecord> lookupEntityActiveBatchInCurrentTxn(
424424
.collect(Collectors.toList());
425425
}
426426

427-
/** {@inheritDoc} */
428-
@Override
429-
public @Nonnull Page<EntityNameLookupRecord> listEntitiesInCurrentTxn(
430-
@Nonnull PolarisCallContext callCtx,
431-
long catalogId,
432-
long parentId,
433-
@Nonnull PolarisEntityType entityType,
434-
@Nonnull PageToken pageToken) {
435-
return this.listEntitiesInCurrentTxn(
436-
callCtx, catalogId, parentId, entityType, Predicates.alwaysTrue(), pageToken);
437-
}
438-
439-
@Override
440-
public @Nonnull Page<EntityNameLookupRecord> listEntitiesInCurrentTxn(
441-
@Nonnull PolarisCallContext callCtx,
442-
long catalogId,
443-
long parentId,
444-
@Nonnull PolarisEntityType entityType,
445-
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
446-
@Nonnull PageToken pageToken) {
447-
// full range scan under the parent for that type
448-
return this.listEntitiesInCurrentTxn(
449-
callCtx,
450-
catalogId,
451-
parentId,
452-
entityType,
453-
entityFilter,
454-
entity ->
455-
new EntityNameLookupRecord(
456-
entity.getCatalogId(),
457-
entity.getId(),
458-
entity.getParentId(),
459-
entity.getName(),
460-
entity.getTypeCode(),
461-
entity.getSubTypeCode()),
462-
pageToken);
463-
}
464-
465427
@Override
466428
public @Nonnull <T> Page<T> listEntitiesInCurrentTxn(
467429
@Nonnull PolarisCallContext callCtx,
468430
long catalogId,
469431
long parentId,
470432
@Nonnull PolarisEntityType entityType,
433+
@Nonnull PolarisEntitySubType entitySubType,
471434
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
472435
@Nonnull Function<PolarisBaseEntity, T> transformer,
473436
@Nonnull PageToken pageToken) {
474437
// full range scan under the parent for that type
475438
Stream<PolarisBaseEntity> data =
476439
this.store
477440
.lookupFullEntitiesActive(
478-
localSession.get(), catalogId, parentId, entityType, pageToken)
441+
localSession.get(), catalogId, parentId, entityType, entitySubType, pageToken)
479442
.stream()
480443
.map(ModelEntity::toEntity)
481444
.filter(entityFilter);

‎persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.apache.polaris.core.entity.PolarisEntitiesActiveKey;
3333
import org.apache.polaris.core.entity.PolarisEntityCore;
3434
import org.apache.polaris.core.entity.PolarisEntityId;
35+
import org.apache.polaris.core.entity.PolarisEntitySubType;
3536
import org.apache.polaris.core.entity.PolarisEntityType;
3637
import org.apache.polaris.core.entity.PolarisGrantRecord;
3738
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
@@ -289,6 +290,7 @@ List<ModelEntity> lookupFullEntitiesActive(
289290
long catalogId,
290291
long parentId,
291292
@Nonnull PolarisEntityType entityType,
293+
@Nonnull PolarisEntitySubType entitySubType,
292294
@Nonnull PageToken pageToken) {
293295
diagnosticServices.check(session != null, "session_is_null");
294296
checkInitialized();
@@ -298,6 +300,10 @@ List<ModelEntity> lookupFullEntitiesActive(
298300
"SELECT m from ModelEntity m where"
299301
+ " m.catalogId=:catalogId and m.parentId=:parentId and m.typeCode=:typeCode";
300302

303+
if (entitySubType != PolarisEntitySubType.ANY_SUBTYPE) {
304+
hql += " and m.subTypeCode=:subTypeCode";
305+
}
306+
301307
var entityIdToken = pageToken.valueAs(EntityIdToken.class);
302308
if (entityIdToken.isPresent()) {
303309
hql += " and m.id > :tokenId";
@@ -314,6 +320,10 @@ List<ModelEntity> lookupFullEntitiesActive(
314320
.setParameter("parentId", parentId)
315321
.setParameter("typeCode", entityType.getCode());
316322

323+
if (entitySubType != PolarisEntitySubType.ANY_SUBTYPE) {
324+
query.setParameter("subTypeCode", entitySubType.getCode());
325+
}
326+
317327
if (entityIdToken.isPresent()) {
318328
long tokenId = entityIdToken.get().entityId();
319329
query = query.setParameter("tokenId", tokenId);

‎persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java‎

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.apache.polaris.core.entity.PolarisEntity;
4545
import org.apache.polaris.core.entity.PolarisEntityCore;
4646
import org.apache.polaris.core.entity.PolarisEntityId;
47+
import org.apache.polaris.core.entity.PolarisEntitySubType;
4748
import org.apache.polaris.core.entity.PolarisEntityType;
4849
import org.apache.polaris.core.entity.PolarisGrantRecord;
4950
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
@@ -427,43 +428,28 @@ public Page<EntityNameLookupRecord> listEntities(
427428
long catalogId,
428429
long parentId,
429430
@Nonnull PolarisEntityType entityType,
431+
@Nonnull PolarisEntitySubType entitySubType,
430432
@Nonnull PageToken pageToken) {
433+
// TODO: only fetch the properties required for creating an EntityNameLookupRecord
431434
return listEntities(
432435
callCtx,
433436
catalogId,
434437
parentId,
435438
entityType,
439+
entitySubType,
436440
entity -> true,
437441
EntityNameLookupRecord::new,
438442
pageToken);
439443
}
440444

441-
@Nonnull
442-
@Override
443-
public Page<EntityNameLookupRecord> listEntities(
444-
@Nonnull PolarisCallContext callCtx,
445-
long catalogId,
446-
long parentId,
447-
@Nonnull PolarisEntityType entityType,
448-
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
449-
@Nonnull PageToken pageToken) {
450-
return listEntities(
451-
callCtx,
452-
catalogId,
453-
parentId,
454-
entityType,
455-
entityFilter,
456-
EntityNameLookupRecord::new,
457-
pageToken);
458-
}
459-
460445
@Nonnull
461446
@Override
462447
public <T> Page<T> listEntities(
463448
@Nonnull PolarisCallContext callCtx,
464449
long catalogId,
465450
long parentId,
466-
PolarisEntityType entityType,
451+
@Nonnull PolarisEntityType entityType,
452+
@Nonnull PolarisEntitySubType entitySubType,
467453
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
468454
@Nonnull Function<PolarisBaseEntity, T> transformer,
469455
@Nonnull PageToken pageToken) {
@@ -479,6 +465,12 @@ public <T> Page<T> listEntities(
479465
realmId);
480466
Map<String, Object> whereGreater;
481467

468+
if (entitySubType != PolarisEntitySubType.ANY_SUBTYPE) {
469+
Map<String, Object> updatedWhereEquals = new HashMap<>(whereEquals);
470+
updatedWhereEquals.put("sub_type_code", entitySubType.getCode());
471+
whereEquals = updatedWhereEquals;
472+
}
473+
482474
// Limit can't be pushed down, due to client side filtering
483475
// absence of transaction.
484476
String orderByColumnName = null;

‎polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import java.util.Set;
3232
import java.util.concurrent.atomic.AtomicInteger;
3333
import java.util.function.Function;
34-
import java.util.function.Predicate;
3534
import java.util.stream.Collectors;
3635
import org.apache.polaris.core.PolarisCallContext;
3736
import org.apache.polaris.core.entity.AsyncTaskType;
@@ -709,14 +708,8 @@ private void revokeGrantRecord(
709708
? 0L
710709
: catalogPath.get(catalogPath.size() - 1).getId();
711710

712-
// prune the returned list with only entities matching the entity subtype
713-
Predicate<PolarisBaseEntity> filter =
714-
entitySubType != PolarisEntitySubType.ANY_SUBTYPE
715-
? e -> e.getSubTypeCode() == entitySubType.getCode()
716-
: entity -> true;
717-
718711
Page<EntityNameLookupRecord> resultPage =
719-
ms.listEntities(callCtx, catalogId, parentId, entityType, filter, pageToken);
712+
ms.listEntities(callCtx, catalogId, parentId, entityType, entitySubType, pageToken);
720713

721714
// TODO: Use post-validation to enforce consistent view against catalogPath. In the
722715
// meantime, happens-before ordering semantics aren't guaranteed during high-concurrency
@@ -1183,6 +1176,7 @@ private void revokeGrantRecord(
11831176
catalogId,
11841177
catalogId,
11851178
PolarisEntityType.CATALOG_ROLE,
1179+
PolarisEntitySubType.ANY_SUBTYPE,
11861180
entity -> true,
11871181
Function.identity(),
11881182
PageToken.fromLimit(2))
@@ -1504,6 +1498,7 @@ private void revokeGrantRecord(
15041498
PolarisEntityConstants.getRootEntityId(),
15051499
PolarisEntityConstants.getRootEntityId(),
15061500
PolarisEntityType.TASK,
1501+
PolarisEntitySubType.ANY_SUBTYPE,
15071502
entity -> {
15081503
PolarisObjectMapperUtil.TaskExecutionState taskState =
15091504
PolarisObjectMapperUtil.parseTaskState(entity);

‎polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java‎

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.apache.polaris.core.entity.PolarisEntity;
3333
import org.apache.polaris.core.entity.PolarisEntityCore;
3434
import org.apache.polaris.core.entity.PolarisEntityId;
35+
import org.apache.polaris.core.entity.PolarisEntitySubType;
3536
import org.apache.polaris.core.entity.PolarisEntityType;
3637
import org.apache.polaris.core.entity.PolarisGrantRecord;
3738
import org.apache.polaris.core.persistence.pagination.Page;
@@ -275,6 +276,7 @@ List<PolarisChangeTrackingVersions> lookupEntityVersions(
275276
* @param catalogId catalog id for that entity, NULL_ID if the entity is top-level
276277
* @param parentId id of the parent, can be the special 0 value representing the root entity
277278
* @param entityType type of entities to list
279+
* @param entitySubType subType of entities to list (or ANY_SUBTYPE)
278280
* @param pageToken the token to start listing after
279281
* @return the list of entities for the specified list operation
280282
*/
@@ -284,27 +286,7 @@ Page<EntityNameLookupRecord> listEntities(
284286
long catalogId,
285287
long parentId,
286288
@Nonnull PolarisEntityType entityType,
287-
@Nonnull PageToken pageToken);
288-
289-
/**
290-
* List entities where some predicate returns true
291-
*
292-
* @param callCtx call context
293-
* @param catalogId catalog id for that entity, NULL_ID if the entity is top-level
294-
* @param parentId id of the parent, can be the special 0 value representing the root entity
295-
* @param entityType type of entities to list
296-
* @param entityFilter the filter to be applied to each entity. Only entities where the predicate
297-
* returns true are returned in the list
298-
* @param pageToken the token to start listing after
299-
* @return the list of entities for which the predicate returns true
300-
*/
301-
@Nonnull
302-
Page<EntityNameLookupRecord> listEntities(
303-
@Nonnull PolarisCallContext callCtx,
304-
long catalogId,
305-
long parentId,
306-
@Nonnull PolarisEntityType entityType,
307-
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
289+
@Nonnull PolarisEntitySubType entitySubType,
308290
@Nonnull PageToken pageToken);
309291

310292
/**
@@ -314,6 +296,7 @@ Page<EntityNameLookupRecord> listEntities(
314296
* @param catalogId catalog id for that entity, NULL_ID if the entity is top-level
315297
* @param parentId id of the parent, can be the special 0 value representing the root entity
316298
* @param entityType type of entities to list
299+
* @param entitySubType subType of entities to list (or ANY_SUBTYPE)
317300
* @param entityFilter the filter to be applied to each entity. Only entities where the predicate
318301
* returns true are returned in the list
319302
* @param transformer the transformation function applied to the {@link PolarisBaseEntity} before
@@ -326,6 +309,7 @@ <T> Page<T> listEntities(
326309
long catalogId,
327310
long parentId,
328311
@Nonnull PolarisEntityType entityType,
312+
@Nonnull PolarisEntitySubType entitySubType,
329313
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
330314
@Nonnull Function<PolarisBaseEntity, T> transformer,
331315
PageToken pageToken);

‎polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java‎

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.apache.polaris.core.entity.PolarisEntitiesActiveKey;
3232
import org.apache.polaris.core.entity.PolarisEntityCore;
3333
import org.apache.polaris.core.entity.PolarisEntityId;
34+
import org.apache.polaris.core.entity.PolarisEntitySubType;
3435
import org.apache.polaris.core.entity.PolarisEntityType;
3536
import org.apache.polaris.core.entity.PolarisGrantRecord;
3637
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
@@ -359,27 +360,13 @@ public Page<EntityNameLookupRecord> listEntities(
359360
long catalogId,
360361
long parentId,
361362
@Nonnull PolarisEntityType entityType,
362-
@Nonnull PageToken pageToken) {
363-
return runInReadTransaction(
364-
callCtx,
365-
() -> this.listEntitiesInCurrentTxn(callCtx, catalogId, parentId, entityType, pageToken));
366-
}
367-
368-
/** {@inheritDoc} */
369-
@Override
370-
@Nonnull
371-
public Page<EntityNameLookupRecord> listEntities(
372-
@Nonnull PolarisCallContext callCtx,
373-
long catalogId,
374-
long parentId,
375-
@Nonnull PolarisEntityType entityType,
376-
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
363+
@Nonnull PolarisEntitySubType entitySubType,
377364
@Nonnull PageToken pageToken) {
378365
return runInReadTransaction(
379366
callCtx,
380367
() ->
381368
this.listEntitiesInCurrentTxn(
382-
callCtx, catalogId, parentId, entityType, entityFilter, pageToken));
369+
callCtx, catalogId, parentId, entityType, entitySubType, pageToken));
383370
}
384371

385372
/** {@inheritDoc} */
@@ -390,14 +377,22 @@ public <T> Page<T> listEntities(
390377
long catalogId,
391378
long parentId,
392379
@Nonnull PolarisEntityType entityType,
380+
@Nonnull PolarisEntitySubType entitySubType,
393381
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
394382
@Nonnull Function<PolarisBaseEntity, T> transformer,
395383
@Nonnull PageToken pageToken) {
396384
return runInReadTransaction(
397385
callCtx,
398386
() ->
399387
this.listEntitiesInCurrentTxn(
400-
callCtx, catalogId, parentId, entityType, entityFilter, transformer, pageToken));
388+
callCtx,
389+
catalogId,
390+
parentId,
391+
entityType,
392+
entitySubType,
393+
entityFilter,
394+
transformer,
395+
pageToken));
401396
}
402397

403398
/** {@inheritDoc} */

‎polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import java.util.Optional;
3131
import java.util.Set;
3232
import java.util.function.Function;
33-
import java.util.function.Predicate;
3433
import java.util.stream.Collectors;
3534
import org.apache.polaris.core.PolarisCallContext;
3635
import org.apache.polaris.core.entity.AsyncTaskType;
@@ -708,20 +707,14 @@ private void bootstrapPolarisService(
708707
return new ListEntitiesResult(BaseResult.ReturnStatus.CATALOG_PATH_CANNOT_BE_RESOLVED, null);
709708
}
710709

711-
Predicate<PolarisBaseEntity> filter = entity -> true;
712-
// prune the returned list with only entities matching the entity subtype
713-
if (entitySubType != PolarisEntitySubType.ANY_SUBTYPE) {
714-
filter = e -> e.getSubTypeCode() == entitySubType.getCode();
715-
}
716-
717710
// return list of active entities
718711
Page<EntityNameLookupRecord> resultPage =
719712
ms.listEntitiesInCurrentTxn(
720713
callCtx,
721714
resolver.getCatalogIdOrNull(),
722715
resolver.getParentId(),
723716
entityType,
724-
filter,
717+
entitySubType,
725718
pageToken);
726719

727720
// done
@@ -1385,6 +1378,7 @@ private void bootstrapPolarisService(
13851378
catalogId,
13861379
catalogId,
13871380
PolarisEntityType.CATALOG_ROLE,
1381+
PolarisEntitySubType.ANY_SUBTYPE,
13881382
entity -> true,
13891383
Function.identity(),
13901384
PageToken.fromLimit(2))
@@ -1958,6 +1952,7 @@ private PolarisEntityResolver resolveSecurableToRoleGrant(
19581952
PolarisEntityConstants.getRootEntityId(),
19591953
PolarisEntityConstants.getRootEntityId(),
19601954
PolarisEntityType.TASK,
1955+
PolarisEntitySubType.ANY_SUBTYPE,
19611956
entity -> {
19621957
PolarisObjectMapperUtil.TaskExecutionState taskState =
19631958
PolarisObjectMapperUtil.parseTaskState(entity);

0 commit comments

Comments
 (0)