eric-maynard commented on code in PR #273:
URL: https://github.com/apache/polaris/pull/273#discussion_r2017896170
##########
extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java:
##########
@@ -441,27 +445,57 @@ public List<EntityNameLookupRecord>
lookupEntityActiveBatchInCurrentTxn(
entity.getParentId(),
entity.getName(),
entity.getTypeCode(),
- entity.getSubTypeCode()));
+ entity.getSubTypeCode()),
+ pageToken);
}
@Override
- public @Nonnull <T> List<T> listEntitiesInCurrentTxn(
+ public @Nonnull <T> PolarisPage<T> listEntitiesInCurrentTxn(
@Nonnull PolarisCallContext callCtx,
long catalogId,
long parentId,
@Nonnull PolarisEntityType entityType,
- int limit,
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
- @Nonnull Function<PolarisBaseEntity, T> transformer) {
- // full range scan under the parent for that type
- return this.store
- .lookupFullEntitiesActive(localSession.get(), catalogId, parentId,
entityType)
- .stream()
- .map(ModelEntity::toEntity)
- .filter(entityFilter)
- .limit(limit)
- .map(transformer)
- .collect(Collectors.toList());
+ @Nonnull Function<PolarisBaseEntity, T> transformer,
+ @Nonnull PageToken pageToken) {
+ List<T> data;
+ if (entityFilter.equals(Predicates.alwaysTrue())) {
+ // In this case, we can push the filter down into the query
+ data =
+ this.store
+ .lookupFullEntitiesActive(
+ localSession.get(), catalogId, parentId, entityType,
pageToken)
+ .stream()
+ .map(ModelEntity::toEntity)
+ .filter(entityFilter)
+ .map(transformer)
+ .collect(Collectors.toList());
+ } else {
+ // In this case, we cannot push the filter down into the query. We must
therefore remove
+ // the page size limit from the PageToken and filter on the client side.
+ // TODO Implement a generic predicate that can be pushed down into
different metastores
+ PageToken unlimitedPageSizeToken =
pageToken.withPageSize(Integer.MAX_VALUE);
+ List<ModelEntity> rawData =
+ this.store.lookupFullEntitiesActive(
+ localSession.get(), catalogId, parentId, entityType,
unlimitedPageSizeToken);
Review Comment:
Looking into this a little more, EclipseLink does support
`TypedQuery.getResultStream`, so maybe it's as simple as just refactoring
`lookupFullEntitiesActive` to return a Stream instead of a List. I'm not sure
how well that actually performs though; we'd want to benchmark it.
--
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: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
