Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
package org.apache.polaris.extension.persistence.impl.eclipselink;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Predicates;
import jakarta.annotation.Nonnull;
import jakarta.annotation.Nullable;
import jakarta.persistence.EntityManager;
Expand Down Expand Up @@ -49,6 +48,7 @@
import org.apache.polaris.core.entity.PolarisEntity;
import org.apache.polaris.core.entity.PolarisEntityCore;
import org.apache.polaris.core.entity.PolarisEntityId;
import org.apache.polaris.core.entity.PolarisEntitySubType;
import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.entity.PolarisGrantRecord;
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
Expand Down Expand Up @@ -424,58 +424,21 @@ public List<EntityNameLookupRecord> lookupEntityActiveBatchInCurrentTxn(
.collect(Collectors.toList());
}

/** {@inheritDoc} */
@Override
public @Nonnull Page<EntityNameLookupRecord> listEntitiesInCurrentTxn(
@Nonnull PolarisCallContext callCtx,
long catalogId,
long parentId,
@Nonnull PolarisEntityType entityType,
@Nonnull PageToken pageToken) {
return this.listEntitiesInCurrentTxn(
callCtx, catalogId, parentId, entityType, Predicates.alwaysTrue(), pageToken);
}

@Override
public @Nonnull Page<EntityNameLookupRecord> listEntitiesInCurrentTxn(
@Nonnull PolarisCallContext callCtx,
long catalogId,
long parentId,
@Nonnull PolarisEntityType entityType,
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
@Nonnull PageToken pageToken) {
// full range scan under the parent for that type
return this.listEntitiesInCurrentTxn(
callCtx,
catalogId,
parentId,
entityType,
entityFilter,
entity ->
new EntityNameLookupRecord(
entity.getCatalogId(),
entity.getId(),
entity.getParentId(),
entity.getName(),
entity.getTypeCode(),
entity.getSubTypeCode()),
pageToken);
}

@Override
public @Nonnull <T> Page<T> listEntitiesInCurrentTxn(
@Nonnull PolarisCallContext callCtx,
long catalogId,
long parentId,
@Nonnull PolarisEntityType entityType,
@Nonnull PolarisEntitySubType entitySubType,
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
@Nonnull Function<PolarisBaseEntity, T> transformer,
@Nonnull PageToken pageToken) {
// full range scan under the parent for that type
Stream<PolarisBaseEntity> data =
this.store
.lookupFullEntitiesActive(
localSession.get(), catalogId, parentId, entityType, pageToken)
localSession.get(), catalogId, parentId, entityType, entitySubType, pageToken)
.stream()
.map(ModelEntity::toEntity)
.filter(entityFilter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.apache.polaris.core.entity.PolarisEntitiesActiveKey;
import org.apache.polaris.core.entity.PolarisEntityCore;
import org.apache.polaris.core.entity.PolarisEntityId;
import org.apache.polaris.core.entity.PolarisEntitySubType;
import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.entity.PolarisGrantRecord;
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
Expand Down Expand Up @@ -289,6 +290,7 @@ List<ModelEntity> lookupFullEntitiesActive(
long catalogId,
long parentId,
@Nonnull PolarisEntityType entityType,
@Nonnull PolarisEntitySubType entitySubType,
@Nonnull PageToken pageToken) {
diagnosticServices.check(session != null, "session_is_null");
checkInitialized();
Expand All @@ -298,6 +300,10 @@ List<ModelEntity> lookupFullEntitiesActive(
"SELECT m from ModelEntity m where"
+ " m.catalogId=:catalogId and m.parentId=:parentId and m.typeCode=:typeCode";

if (entitySubType != PolarisEntitySubType.ANY_SUBTYPE) {
hql += " and m.subTypeCode=:subTypeCode";
}

var entityIdToken = pageToken.valueAs(EntityIdToken.class);
if (entityIdToken.isPresent()) {
hql += " and m.id > :tokenId";
Expand All @@ -314,6 +320,10 @@ List<ModelEntity> lookupFullEntitiesActive(
.setParameter("parentId", parentId)
.setParameter("typeCode", entityType.getCode());

if (entitySubType != PolarisEntitySubType.ANY_SUBTYPE) {
query.setParameter("subTypeCode", entitySubType.getCode());
}

if (entityIdToken.isPresent()) {
long tokenId = entityIdToken.get().entityId();
query = query.setParameter("tokenId", tokenId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.apache.polaris.core.entity.PolarisEntity;
import org.apache.polaris.core.entity.PolarisEntityCore;
import org.apache.polaris.core.entity.PolarisEntityId;
import org.apache.polaris.core.entity.PolarisEntitySubType;
import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.entity.PolarisGrantRecord;
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
Expand Down Expand Up @@ -427,43 +428,28 @@ public Page<EntityNameLookupRecord> listEntities(
long catalogId,
long parentId,
@Nonnull PolarisEntityType entityType,
@Nonnull PolarisEntitySubType entitySubType,
@Nonnull PageToken pageToken) {
// TODO: only fetch the properties required for creating an EntityNameLookupRecord
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To help us remember as well, can we add and update the underlying CONSTRAINT clause to make the implicit index INCLUDE the subTypeCode to this TODO?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll take a stab at this as an immediate follow-up or file it as a separate issue where i will take note of this additional "index coverage" requirement

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filed the ticket: #2352

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return listEntities(
callCtx,
catalogId,
parentId,
entityType,
entitySubType,
entity -> true,
EntityNameLookupRecord::new,
pageToken);
}

@Nonnull
@Override
public Page<EntityNameLookupRecord> listEntities(
@Nonnull PolarisCallContext callCtx,
long catalogId,
long parentId,
@Nonnull PolarisEntityType entityType,
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
@Nonnull PageToken pageToken) {
return listEntities(
callCtx,
catalogId,
parentId,
entityType,
entityFilter,
EntityNameLookupRecord::new,
pageToken);
}

@Nonnull
@Override
public <T> Page<T> listEntities(
@Nonnull PolarisCallContext callCtx,
long catalogId,
long parentId,
PolarisEntityType entityType,
@Nonnull PolarisEntityType entityType,
@Nonnull PolarisEntitySubType entitySubType,
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
@Nonnull Function<PolarisBaseEntity, T> transformer,
@Nonnull PageToken pageToken) {
Expand All @@ -479,6 +465,12 @@ public <T> Page<T> listEntities(
realmId);
Map<String, Object> whereGreater;

if (entitySubType != PolarisEntitySubType.ANY_SUBTYPE) {
Map<String, Object> updatedWhereEquals = new HashMap<>(whereEquals);
updatedWhereEquals.put("sub_type_code", entitySubType.getCode());
whereEquals = updatedWhereEquals;
}

// Limit can't be pushed down, due to client side filtering
// absence of transaction.
String orderByColumnName = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.entity.AsyncTaskType;
Expand Down Expand Up @@ -709,14 +708,8 @@ private void revokeGrantRecord(
? 0L
: catalogPath.get(catalogPath.size() - 1).getId();

// prune the returned list with only entities matching the entity subtype
Predicate<PolarisBaseEntity> filter =
entitySubType != PolarisEntitySubType.ANY_SUBTYPE
? e -> e.getSubTypeCode() == entitySubType.getCode()
: entity -> true;

Page<EntityNameLookupRecord> resultPage =
ms.listEntities(callCtx, catalogId, parentId, entityType, filter, pageToken);
ms.listEntities(callCtx, catalogId, parentId, entityType, entitySubType, pageToken);

// TODO: Use post-validation to enforce consistent view against catalogPath. In the
// meantime, happens-before ordering semantics aren't guaranteed during high-concurrency
Expand Down Expand Up @@ -1183,6 +1176,7 @@ private void revokeGrantRecord(
catalogId,
catalogId,
PolarisEntityType.CATALOG_ROLE,
PolarisEntitySubType.ANY_SUBTYPE,
entity -> true,
Function.identity(),
PageToken.fromLimit(2))
Expand Down Expand Up @@ -1504,6 +1498,7 @@ private void revokeGrantRecord(
PolarisEntityConstants.getRootEntityId(),
PolarisEntityConstants.getRootEntityId(),
PolarisEntityType.TASK,
PolarisEntitySubType.ANY_SUBTYPE,
entity -> {
PolarisObjectMapperUtil.TaskExecutionState taskState =
PolarisObjectMapperUtil.parseTaskState(entity);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.apache.polaris.core.entity.PolarisEntity;
import org.apache.polaris.core.entity.PolarisEntityCore;
import org.apache.polaris.core.entity.PolarisEntityId;
import org.apache.polaris.core.entity.PolarisEntitySubType;
import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.entity.PolarisGrantRecord;
import org.apache.polaris.core.persistence.pagination.Page;
Expand Down Expand Up @@ -275,6 +276,7 @@ List<PolarisChangeTrackingVersions> lookupEntityVersions(
* @param catalogId catalog id for that entity, NULL_ID if the entity is top-level
* @param parentId id of the parent, can be the special 0 value representing the root entity
* @param entityType type of entities to list
* @param entitySubType subType of entities to list (or ANY_SUBTYPE)
* @param pageToken the token to start listing after
* @return the list of entities for the specified list operation
*/
Expand All @@ -284,27 +286,7 @@ Page<EntityNameLookupRecord> listEntities(
long catalogId,
long parentId,
@Nonnull PolarisEntityType entityType,
@Nonnull PageToken pageToken);

/**
* List entities where some predicate returns true
*
* @param callCtx call context
* @param catalogId catalog id for that entity, NULL_ID if the entity is top-level
* @param parentId id of the parent, can be the special 0 value representing the root entity
* @param entityType type of entities to list
* @param entityFilter the filter to be applied to each entity. Only entities where the predicate
* returns true are returned in the list
* @param pageToken the token to start listing after
* @return the list of entities for which the predicate returns true
*/
@Nonnull
Page<EntityNameLookupRecord> listEntities(
@Nonnull PolarisCallContext callCtx,
long catalogId,
long parentId,
@Nonnull PolarisEntityType entityType,
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
@Nonnull PolarisEntitySubType entitySubType,
@Nonnull PageToken pageToken);

/**
Expand All @@ -314,6 +296,7 @@ Page<EntityNameLookupRecord> listEntities(
* @param catalogId catalog id for that entity, NULL_ID if the entity is top-level
* @param parentId id of the parent, can be the special 0 value representing the root entity
* @param entityType type of entities to list
* @param entitySubType subType of entities to list (or ANY_SUBTYPE)
* @param entityFilter the filter to be applied to each entity. Only entities where the predicate
* returns true are returned in the list
* @param transformer the transformation function applied to the {@link PolarisBaseEntity} before
Expand All @@ -326,6 +309,7 @@ <T> Page<T> listEntities(
long catalogId,
long parentId,
@Nonnull PolarisEntityType entityType,
@Nonnull PolarisEntitySubType entitySubType,
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
@Nonnull Function<PolarisBaseEntity, T> transformer,
PageToken pageToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.apache.polaris.core.entity.PolarisEntitiesActiveKey;
import org.apache.polaris.core.entity.PolarisEntityCore;
import org.apache.polaris.core.entity.PolarisEntityId;
import org.apache.polaris.core.entity.PolarisEntitySubType;
import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.entity.PolarisGrantRecord;
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
Expand Down Expand Up @@ -359,27 +360,13 @@ public Page<EntityNameLookupRecord> listEntities(
long catalogId,
long parentId,
@Nonnull PolarisEntityType entityType,
@Nonnull PageToken pageToken) {
return runInReadTransaction(
callCtx,
() -> this.listEntitiesInCurrentTxn(callCtx, catalogId, parentId, entityType, pageToken));
}

/** {@inheritDoc} */
@Override
@Nonnull
public Page<EntityNameLookupRecord> listEntities(
@Nonnull PolarisCallContext callCtx,
long catalogId,
long parentId,
@Nonnull PolarisEntityType entityType,
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
@Nonnull PolarisEntitySubType entitySubType,
@Nonnull PageToken pageToken) {
return runInReadTransaction(
callCtx,
() ->
this.listEntitiesInCurrentTxn(
callCtx, catalogId, parentId, entityType, entityFilter, pageToken));
callCtx, catalogId, parentId, entityType, entitySubType, pageToken));
}

/** {@inheritDoc} */
Expand All @@ -390,14 +377,22 @@ public <T> Page<T> listEntities(
long catalogId,
long parentId,
@Nonnull PolarisEntityType entityType,
@Nonnull PolarisEntitySubType entitySubType,
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
@Nonnull Function<PolarisBaseEntity, T> transformer,
@Nonnull PageToken pageToken) {
return runInReadTransaction(
callCtx,
() ->
this.listEntitiesInCurrentTxn(
callCtx, catalogId, parentId, entityType, entityFilter, transformer, pageToken));
callCtx,
catalogId,
parentId,
entityType,
entitySubType,
entityFilter,
transformer,
pageToken));
}

/** {@inheritDoc} */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.entity.AsyncTaskType;
Expand Down Expand Up @@ -710,20 +709,14 @@ private void bootstrapPolarisService(
return new ListEntitiesResult(BaseResult.ReturnStatus.CATALOG_PATH_CANNOT_BE_RESOLVED, null);
}

Predicate<PolarisBaseEntity> filter = entity -> true;
// prune the returned list with only entities matching the entity subtype
if (entitySubType != PolarisEntitySubType.ANY_SUBTYPE) {
filter = e -> e.getSubTypeCode() == entitySubType.getCode();
}

// return list of active entities
Page<EntityNameLookupRecord> resultPage =
ms.listEntitiesInCurrentTxn(
callCtx,
resolver.getCatalogIdOrNull(),
resolver.getParentId(),
entityType,
filter,
entitySubType,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that due to this change calls to PolarisMetaStoreManager.listEntities like here:

private Page<TableIdentifier> listTableLike(
PolarisEntitySubType subType, Namespace namespace, PageToken pageToken) {
PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getResolvedPath(namespace);
if (resolvedEntities == null) {
// Illegal state because the namespace should've already been in the static resolution set.
throw new IllegalStateException(
String.format("Failed to fetch resolved namespace '%s'", namespace));
}
List<PolarisEntity> catalogPath = resolvedEntities.getRawFullPath();
ListEntitiesResult listResult =
getMetaStoreManager()
.listEntities(
getCurrentPolarisContext(),
PolarisEntity.toCoreList(catalogPath),
PolarisEntityType.TABLE_LIKE,
subType,
pageToken);

no longer have to load the full PolarisBaseEntity to apply the Predicate<PolarisBaseEntity> and thus they can actually take advantage of optimized implementations of Page<EntityNameLookupRecord> listEntities in BasePersistence that load only the required properties.

pageToken);

// done
Expand Down Expand Up @@ -1387,6 +1380,7 @@ private void bootstrapPolarisService(
catalogId,
catalogId,
PolarisEntityType.CATALOG_ROLE,
PolarisEntitySubType.ANY_SUBTYPE,
entity -> true,
Function.identity(),
PageToken.fromLimit(2))
Expand Down Expand Up @@ -1960,6 +1954,7 @@ private PolarisEntityResolver resolveSecurableToRoleGrant(
PolarisEntityConstants.getRootEntityId(),
PolarisEntityConstants.getRootEntityId(),
PolarisEntityType.TASK,
PolarisEntitySubType.ANY_SUBTYPE,
entity -> {
PolarisObjectMapperUtil.TaskExecutionState taskState =
PolarisObjectMapperUtil.parseTaskState(entity);
Expand Down
Loading