eric-maynard commented on code in PR #273:
URL: https://github.com/apache/polaris/pull/273#discussion_r2021926372
##########
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:
I don't think this needs to be transactionally consistent. You can miss an
entity, but you will never get the same entity twice.
Entity ID is guaranteed to always be unique.
I suppose there is a case where you transactionally add some table and drop
some other table that was already returned in an existing page -- the list
results in aggregate will be inconsistent with that transaction. But I think
this is outside the realm of what we need to support for the time being. I
would probably not want to fail either the list operation or the table update
operation to ensure consistency when clients who care about transactional
consistency have the option to list without a page token.
--
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]
