This is an automated email from the ASF dual-hosted git repository. dhuo pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push: new b49cbc593 Add PolarisMetaStoreManager.loadEntities (#2290) b49cbc593 is described below commit b49cbc59382fb5f98b3b46240cfdcf92d5608efa Author: Christopher Lambert <xn...@gmx.de> AuthorDate: Wed Aug 20 23:02:54 2025 +0200 Add PolarisMetaStoreManager.loadEntities (#2290) * Add PolarisMetaStoreManager.loadEntities currently `PolarisMetaStoreManager.listEntities` only exposes a limited subset of the underlying `BasePersistence.listEntities` functionality. most of the callers have to post-process the `EntityNameLookupRecord` of `ListEntitiesResult` and call `PolarisMetaStoreManager.loadEntity` on the individual items sequentually to transform and filter them. this is bad for the following reasons: - suboptimal performance as we run N+1 queries to basically load every entity twice from the persistence backend - suffering from race-conditions when entities get dropped between the `listEntities` and `loadEntity` call - a lot of repeated code in all the callers (of which only some are dealing with the race-condition by filtering out null values) as a solution we add `PolarisMetaStoreManager.loadEntities` that takes advantage of the already existing `BasePersistence` methods. we rename one of the `listEntities` methods to `loadEntities` for consistency. since many callers dont need paging and want the result as a list, we add `PolarisMetaStoreManager.loadEntitiesAll` as a convenient wrapper. we also remove the `PolarisEntity.nameAndId` method as callers who only need name and id should not be loading the full entity to begin with. note we rework `testCatalogNotReturnedWhenDeletedAfterListBeforeGet` from `ManagementServiceTest` because the simulated race-condition scenario can no longer happen. * review: remove filter + transformer params * review: add TODO in listPolicies * review: improve javadoc --- .../PolarisEclipseLinkMetaStoreSessionImpl.java | 2 +- .../relational/jdbc/JdbcBasePersistenceImpl.java | 4 +- .../apache/polaris/core/entity/PolarisEntity.java | 5 -- .../AtomicOperationMetaStoreManager.java | 41 ++++++++++- .../polaris/core/persistence/BasePersistence.java | 12 ++-- .../core/persistence/PolarisMetaStoreManager.java | 46 +++++++++++- .../TransactionWorkspaceMetaStoreManager.java | 14 +++- .../AbstractTransactionalPersistence.java | 4 +- .../TransactionalMetaStoreManagerImpl.java | 53 +++++++++++++- .../transactional/TransactionalPersistence.java | 6 +- .../TreeMapTransactionalPersistenceImpl.java | 12 +--- .../BasePolarisMetaStoreManagerTest.java | 33 +++------ .../polaris/service/admin/PolarisAdminService.java | 82 ++++++++-------------- .../service/catalog/iceberg/IcebergCatalog.java | 10 ++- .../service/catalog/policy/PolicyCatalog.java | 39 +++------- .../service/admin/ManagementServiceTest.java | 27 +++---- 16 files changed, 223 insertions(+), 167 deletions(-) diff --git a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index 9929c2da3..75c4c4ad4 100644 --- a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -428,7 +428,7 @@ public class PolarisEclipseLinkMetaStoreSessionImpl extends AbstractTransactiona } @Override - public @Nonnull <T> Page<T> listEntitiesInCurrentTxn( + public @Nonnull <T> Page<T> loadEntitiesInCurrentTxn( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index 1babeb03e..71f1d2e7a 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -431,7 +431,7 @@ public class JdbcBasePersistenceImpl implements BasePersistence, IntegrationPers @Nonnull PolarisEntitySubType entitySubType, @Nonnull PageToken pageToken) { // TODO: only fetch the properties required for creating an EntityNameLookupRecord - return listEntities( + return loadEntities( callCtx, catalogId, parentId, @@ -444,7 +444,7 @@ public class JdbcBasePersistenceImpl implements BasePersistence, IntegrationPers @Nonnull @Override - public <T> Page<T> listEntities( + public <T> Page<T> loadEntities( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java index 1d5def481..5dccbe1bf 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java @@ -223,11 +223,6 @@ public class PolarisEntity extends PolarisBaseEntity { return PolarisEntitySubType.fromCode(getSubTypeCode()); } - @JsonIgnore - public NameAndId nameAndId() { - return new NameAndId(name, id); - } - @Override public String toString() { return "name=" diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index 8af4c654d..7d7a26c48 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -717,10 +717,45 @@ public class AtomicOperationMetaStoreManager extends BaseMetaStoreManager { // with sensitive data; but the window of inconsistency is only the duration of a single // in-flight request (the cache-based resolution follows a different path entirely). - // done return ListEntitiesResult.fromPage(resultPage); } + /** {@inheritDoc} */ + @Override + public @Nonnull Page<PolarisBaseEntity> loadEntities( + @Nonnull PolarisCallContext callCtx, + @Nullable List<PolarisEntityCore> catalogPath, + @Nonnull PolarisEntityType entityType, + @Nonnull PolarisEntitySubType entitySubType, + @Nonnull PageToken pageToken) { + // get meta store we should be using + BasePersistence ms = callCtx.getMetaStore(); + + // return list of active entities + // TODO: Clean up shared logic for catalogId/parentId + long catalogId = catalogPath == null || catalogPath.isEmpty() ? 0L : catalogPath.get(0).getId(); + long parentId = + catalogPath == null || catalogPath.isEmpty() + ? 0L + : catalogPath.get(catalogPath.size() - 1).getId(); + + // TODO: Use post-validation to enforce consistent view against catalogPath. In the + // meantime, happens-before ordering semantics aren't guaranteed during high-concurrency + // race conditions, such as first revoking a grant on a namespace before adding a table + // with sensitive data; but the window of inconsistency is only the duration of a single + // in-flight request (the cache-based resolution follows a different path entirely). + + return ms.loadEntities( + callCtx, + catalogId, + parentId, + entityType, + entitySubType, + entity -> true, + Function.identity(), + pageToken); + } + /** {@inheritDoc} */ @Override public @Nonnull CreatePrincipalResult createPrincipal( @@ -1166,7 +1201,7 @@ public class AtomicOperationMetaStoreManager extends BaseMetaStoreManager { // get the list of catalog roles, at most 2 List<PolarisBaseEntity> catalogRoles = - ms.listEntities( + ms.loadEntities( callCtx, catalogId, catalogId, @@ -1488,7 +1523,7 @@ public class AtomicOperationMetaStoreManager extends BaseMetaStoreManager { // find all available tasks Page<PolarisBaseEntity> availableTasks = - ms.listEntities( + ms.loadEntities( callCtx, PolarisEntityConstants.getRootEntityId(), PolarisEntityConstants.getRootEntityId(), diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java index 1e8d2abf8..6134e5af6 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java @@ -270,7 +270,8 @@ public interface BasePersistence extends PolicyMappingPersistence { @Nonnull PolarisCallContext callCtx, List<PolarisEntityId> entityIds); /** - * List all entities of the specified type which are child entities of the specified parent + * List lightweight information of entities matching the given criteria with pagination. If all + * properties of the entity are required,use {@link #loadEntities} instead. * * @param callCtx call context * @param catalogId catalog id for that entity, NULL_ID if the entity is top-level @@ -278,7 +279,7 @@ public interface BasePersistence extends PolicyMappingPersistence { * @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 + * @return the paged list of matching entities */ @Nonnull Page<EntityNameLookupRecord> listEntities( @@ -290,7 +291,8 @@ public interface BasePersistence extends PolicyMappingPersistence { @Nonnull PageToken pageToken); /** - * List entities where some predicate returns true and transform the entities with a function + * Load full entities matching the given criteria with pagination and transformation. If only the + * entity name/id/type is required, use {@link #listEntities} instead. * * @param callCtx call context * @param catalogId catalog id for that entity, NULL_ID if the entity is top-level @@ -301,10 +303,10 @@ public interface BasePersistence extends PolicyMappingPersistence { * returns true are returned in the list * @param transformer the transformation function applied to the {@link PolarisBaseEntity} before * returning - * @return the list of entities for which the predicate returns true + * @return the paged list of matching entities after transformation */ @Nonnull - <T> Page<T> listEntities( + <T> Page<T> loadEntities( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java index 67175e21f..ffe3ca2a2 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java @@ -47,6 +47,7 @@ import org.apache.polaris.core.persistence.dao.entity.EntityWithPath; import org.apache.polaris.core.persistence.dao.entity.GenerateEntityIdResult; import org.apache.polaris.core.persistence.dao.entity.ListEntitiesResult; import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; +import org.apache.polaris.core.persistence.pagination.Page; import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.policy.PolarisPolicyMappingManager; import org.apache.polaris.core.storage.PolarisCredentialVendor; @@ -110,8 +111,8 @@ public interface PolarisMetaStoreManager @Nonnull String name); /** - * List all entities of the specified type under the specified catalogPath. If the catalogPath is - * null, listed entities will be top-level entities like catalogs. + * List lightweight information about entities matching the given criteria. If all properties of + * the entity are required,use {@link #loadEntities} instead. * * @param callCtx call context * @param catalogPath path inside a catalog. If null or empty, the entities to list are top-level, @@ -129,6 +130,47 @@ public interface PolarisMetaStoreManager @Nonnull PolarisEntitySubType entitySubType, @Nonnull PageToken pageToken); + /** + * Load full entities matching the given criteria with pagination. If only the entity name/id/type + * is required, use {@link #listEntities} instead. If no pagination is required, use {@link + * #loadEntitiesAll} instead. + * + * @param callCtx call context + * @param catalogPath path inside a catalog. If null or empty, the entities to list are top-level, + * like catalogs + * @param entityType type of entities to list + * @param entitySubType subType of entities to list (or ANY_SUBTYPE) + * @return paged list of matching entities + */ + @Nonnull + Page<PolarisBaseEntity> loadEntities( + @Nonnull PolarisCallContext callCtx, + @Nullable List<PolarisEntityCore> catalogPath, + @Nonnull PolarisEntityType entityType, + @Nonnull PolarisEntitySubType entitySubType, + @Nonnull PageToken pageToken); + + /** + * Load full entities matching the given criteria into an unpaged list. If pagination is required + * use {@link #loadEntities} instead. If only the entity name/id/type is required, use {@link + * #listEntities} instead. + * + * @param callCtx call context + * @param catalogPath path inside a catalog. If null or empty, the entities to list are top-level, + * like catalogs + * @param entityType type of entities to list + * @param entitySubType subType of entities to list (or ANY_SUBTYPE) + * @return list of all matching entities + */ + default @Nonnull List<PolarisBaseEntity> loadEntitiesAll( + @Nonnull PolarisCallContext callCtx, + @Nullable List<PolarisEntityCore> catalogPath, + @Nonnull PolarisEntityType entityType, + @Nonnull PolarisEntitySubType entitySubType) { + return loadEntities(callCtx, catalogPath, entityType, entitySubType, PageToken.readEverything()) + .items(); + } + /** * Generate a new unique id that can be used by the Polaris client when it needs to create a new * entity diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java index f707d1c29..2671ad98b 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java @@ -53,6 +53,7 @@ import org.apache.polaris.core.persistence.dao.entity.PrincipalSecretsResult; import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult; import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult; +import org.apache.polaris.core.persistence.pagination.Page; import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.policy.PolicyEntity; import org.apache.polaris.core.policy.PolicyType; @@ -119,7 +120,7 @@ public class TransactionWorkspaceMetaStoreManager implements PolarisMetaStoreMan } @Override - public ListEntitiesResult listEntities( + public @Nonnull ListEntitiesResult listEntities( @Nonnull PolarisCallContext callCtx, @Nullable List<PolarisEntityCore> catalogPath, @Nonnull PolarisEntityType entityType, @@ -129,6 +130,17 @@ public class TransactionWorkspaceMetaStoreManager implements PolarisMetaStoreMan return null; } + @Override + public @Nonnull Page<PolarisBaseEntity> loadEntities( + @Nonnull PolarisCallContext callCtx, + @Nullable List<PolarisEntityCore> catalogPath, + @Nonnull PolarisEntityType entityType, + @Nonnull PolarisEntitySubType entitySubType, + @Nonnull PageToken pageToken) { + callCtx.getDiagServices().fail("illegal_method_in_transaction_workspace", "loadEntities"); + return null; + } + @Override public GenerateEntityIdResult generateNewEntityId(@Nonnull PolarisCallContext callCtx) { diagnostics.fail("illegal_method_in_transaction_workspace", "generateNewEntityId"); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java index 138b3f4a7..d5b323fab 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java @@ -383,7 +383,7 @@ public abstract class AbstractTransactionalPersistence implements TransactionalP /** {@inheritDoc} */ @Override @Nonnull - public <T> Page<T> listEntities( + public <T> Page<T> loadEntities( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, @@ -395,7 +395,7 @@ public abstract class AbstractTransactionalPersistence implements TransactionalP return runInReadTransaction( callCtx, () -> - this.listEntitiesInCurrentTxn( + this.loadEntitiesInCurrentTxn( callCtx, catalogId, parentId, diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index ee202d3ca..c187a6375 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -699,7 +699,6 @@ public class TransactionalMetaStoreManagerImpl extends BaseMetaStoreManager { entitySubType, pageToken); - // done return ListEntitiesResult.fromPage(resultPage); } @@ -720,6 +719,54 @@ public class TransactionalMetaStoreManagerImpl extends BaseMetaStoreManager { () -> listEntities(callCtx, ms, catalogPath, entityType, entitySubType, pageToken)); } + /** + * See {@link PolarisMetaStoreManager#loadEntities(PolarisCallContext, List, PolarisEntityType, + * PolarisEntitySubType, PageToken)} + */ + private @Nonnull Page<PolarisBaseEntity> loadEntities( + @Nonnull PolarisCallContext callCtx, + @Nonnull TransactionalPersistence ms, + @Nullable List<PolarisEntityCore> catalogPath, + @Nonnull PolarisEntityType entityType, + @Nonnull PolarisEntitySubType entitySubType, + @Nonnull PageToken pageToken) { + // first resolve again the catalogPath to that entity + PolarisEntityResolver resolver = new PolarisEntityResolver(callCtx, ms, catalogPath); + + // throw if we failed to resolve + if (resolver.isFailure()) { + throw new IllegalArgumentException("Failed to resolve catalogPath: " + catalogPath); + } + + // return list of active entities + return ms.loadEntitiesInCurrentTxn( + callCtx, + resolver.getCatalogIdOrNull(), + resolver.getParentId(), + entityType, + entitySubType, + entity -> true, + Function.identity(), + pageToken); + } + + /** {@inheritDoc} */ + @Override + public @Nonnull Page<PolarisBaseEntity> loadEntities( + @Nonnull PolarisCallContext callCtx, + @Nullable List<PolarisEntityCore> catalogPath, + @Nonnull PolarisEntityType entityType, + @Nonnull PolarisEntitySubType entitySubType, + @Nonnull PageToken pageToken) { + // get meta store we should be using + TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); + + // run operation in a read transaction + return ms.runInReadTransaction( + callCtx, + () -> loadEntities(callCtx, ms, catalogPath, entityType, entitySubType, pageToken)); + } + /** {@link #createPrincipal(PolarisCallContext, PolarisBaseEntity)} */ private @Nonnull CreatePrincipalResult createPrincipal( @Nonnull PolarisCallContext callCtx, @@ -1335,7 +1382,7 @@ public class TransactionalMetaStoreManagerImpl extends BaseMetaStoreManager { // get the list of catalog roles, at most 2 List<PolarisBaseEntity> catalogRoles = - ms.listEntitiesInCurrentTxn( + ms.loadEntitiesInCurrentTxn( callCtx, catalogId, catalogId, @@ -1901,7 +1948,7 @@ public class TransactionalMetaStoreManagerImpl extends BaseMetaStoreManager { // find all available tasks Page<PolarisBaseEntity> availableTasks = - ms.listEntitiesInCurrentTxn( + ms.loadEntitiesInCurrentTxn( callCtx, PolarisEntityConstants.getRootEntityId(), PolarisEntityConstants.getRootEntityId(), diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalPersistence.java index 6eacd62db..3802908b8 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalPersistence.java @@ -214,7 +214,7 @@ public interface TransactionalPersistence @Nonnull PolarisEntityType entityType, @Nonnull PolarisEntitySubType entitySubType, @Nonnull PageToken pageToken) { - return listEntitiesInCurrentTxn( + return loadEntitiesInCurrentTxn( callCtx, catalogId, parentId, @@ -225,9 +225,9 @@ public interface TransactionalPersistence pageToken); } - /** See {@link org.apache.polaris.core.persistence.BasePersistence#listEntities} */ + /** See {@link org.apache.polaris.core.persistence.BasePersistence#loadEntities} */ @Nonnull - <T> Page<T> listEntitiesInCurrentTxn( + <T> Page<T> loadEntitiesInCurrentTxn( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index 7fdd94b9a..651800621 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -288,15 +288,7 @@ public class TreeMapTransactionalPersistenceImpl extends AbstractTransactionalPe entityActiveKey.getName())); // return record - return (entity == null) - ? null - : new EntityNameLookupRecord( - entity.getCatalogId(), - entity.getId(), - entity.getParentId(), - entity.getName(), - entity.getTypeCode(), - entity.getSubTypeCode()); + return entity == null ? null : new EntityNameLookupRecord(entity); } /** {@inheritDoc} */ @@ -312,7 +304,7 @@ public class TreeMapTransactionalPersistenceImpl extends AbstractTransactionalPe } @Override - public @Nonnull <T> Page<T> listEntitiesInCurrentTxn( + public @Nonnull <T> Page<T> loadEntitiesInCurrentTxn( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java index 6adb042ac..336b6b5d0 100644 --- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java +++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java @@ -36,7 +36,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.entity.AsyncTaskType; -import org.apache.polaris.core.entity.EntityNameLookupRecord; import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.entity.PolarisEntity; import org.apache.polaris.core.entity.PolarisEntitySubType; @@ -118,33 +117,17 @@ public abstract class BasePolarisMetaStoreManagerTest { .extracting(PolarisEntity::toCore) .containsExactly(PolarisEntity.toCore(task1), PolarisEntity.toCore(task2)); - List<EntityNameLookupRecord> listedEntities = - metaStoreManager - .listEntities( - polarisTestMetaStoreManager.polarisCallContext, - null, - PolarisEntityType.TASK, - PolarisEntitySubType.NULL_SUBTYPE, - PageToken.readEverything()) - .getEntities(); + List<PolarisBaseEntity> listedEntities = + metaStoreManager.loadEntitiesAll( + polarisTestMetaStoreManager.polarisCallContext, + null, + PolarisEntityType.TASK, + PolarisEntitySubType.NULL_SUBTYPE); Assertions.assertThat(listedEntities) .isNotNull() .hasSize(2) - .containsExactly( - new EntityNameLookupRecord( - task1.getCatalogId(), - task1.getId(), - task1.getParentId(), - task1.getName(), - task1.getTypeCode(), - task1.getSubTypeCode()), - new EntityNameLookupRecord( - task2.getCatalogId(), - task2.getId(), - task2.getParentId(), - task2.getName(), - task2.getTypeCode(), - task2.getSubTypeCode())); + .extracting(PolarisEntity::toCore) + .containsExactly(PolarisEntity.toCore(task1), PolarisEntity.toCore(task2)); } @Test diff --git a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index e817b2404..9462e13d6 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -106,7 +106,6 @@ import org.apache.polaris.core.persistence.dao.entity.DropEntityResult; import org.apache.polaris.core.persistence.dao.entity.EntityResult; import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult; import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult; -import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; import org.apache.polaris.core.persistence.resolver.ResolutionManifestFactory; import org.apache.polaris.core.persistence.resolver.ResolverPath; @@ -208,43 +207,6 @@ public class PolarisAdminService { .map(path -> CatalogRoleEntity.of(path.getRawLeafEntity())); } - private <T> Stream<T> loadEntities( - @Nonnull PolarisEntityType entityType, - @Nonnull PolarisEntitySubType entitySubType, - @Nullable PolarisEntity catalogEntity, - @Nonnull Function<PolarisBaseEntity, T> transformer) { - List<PolarisEntityCore> catalogPath; - long catalogId; - if (catalogEntity == null) { - catalogPath = null; - catalogId = 0; - } else { - catalogPath = PolarisEntity.toCoreList(List.of(catalogEntity)); - catalogId = catalogEntity.getId(); - } - // TODO: add loadEntities method to PolarisMetaStoreManager - // loadEntity may return null due to multiple non-atomic API calls to the persistence layer. - // Specifically, this can happen when a PolarisEntity is returned by listEntities, but cannot be - // loaded afterward because it was purged by another process before it could be loaded. - return metaStoreManager - .listEntities( - getCurrentPolarisContext(), - catalogPath, - entityType, - entitySubType, - PageToken.readEverything()) - .getEntities() - .stream() - .map( - nameAndId -> - metaStoreManager.loadEntity( - getCurrentPolarisContext(), catalogId, nameAndId.getId(), nameAndId.getType())) - .map(PolarisEntity::of) - .filter(Objects::nonNull) - .map(transformer) - .filter(Objects::nonNull); - } - private void authorizeBasicRootOperationOrThrow(PolarisAuthorizableOperation op) { resolutionManifest = resolutionManifestFactory.createResolutionManifest( @@ -983,8 +945,14 @@ public class PolarisAdminService { /** List all catalogs without checking for permission. */ private Stream<CatalogEntity> listCatalogsUnsafe() { - return loadEntities( - PolarisEntityType.CATALOG, PolarisEntitySubType.ANY_SUBTYPE, null, CatalogEntity::of); + return metaStoreManager + .loadEntitiesAll( + getCurrentPolarisContext(), + null, + PolarisEntityType.CATALOG, + PolarisEntitySubType.ANY_SUBTYPE) + .stream() + .map(CatalogEntity::of); } public PrincipalWithCredentials createPrincipal(PolarisEntity entity) { @@ -1152,11 +1120,14 @@ public class PolarisAdminService { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_PRINCIPALS; authorizeBasicRootOperationOrThrow(op); - return loadEntities( - PolarisEntityType.PRINCIPAL, - PolarisEntitySubType.NULL_SUBTYPE, + return metaStoreManager + .loadEntitiesAll( + getCurrentPolarisContext(), null, - PrincipalEntity::of) + PolarisEntityType.PRINCIPAL, + PolarisEntitySubType.NULL_SUBTYPE) + .stream() + .map(PrincipalEntity::of) .map(PrincipalEntity::asPrincipal) .toList(); } @@ -1257,11 +1228,14 @@ public class PolarisAdminService { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_PRINCIPAL_ROLES; authorizeBasicRootOperationOrThrow(op); - return loadEntities( - PolarisEntityType.PRINCIPAL_ROLE, - PolarisEntitySubType.NULL_SUBTYPE, + return metaStoreManager + .loadEntitiesAll( + getCurrentPolarisContext(), null, - PrincipalRoleEntity::of) + PolarisEntityType.PRINCIPAL_ROLE, + PolarisEntitySubType.NULL_SUBTYPE) + .stream() + .map(PrincipalRoleEntity::of) .map(PrincipalRoleEntity::asPrincipalRole) .toList(); } @@ -1381,11 +1355,15 @@ public class PolarisAdminService { PolarisEntity catalogEntity = findCatalogByName(catalogName) .orElseThrow(() -> new NotFoundException("Parent catalog %s not found", catalogName)); - return loadEntities( + List<PolarisEntityCore> catalogPath = PolarisEntity.toCoreList(List.of(catalogEntity)); + return metaStoreManager + .loadEntitiesAll( + getCurrentPolarisContext(), + catalogPath, PolarisEntityType.CATALOG_ROLE, - PolarisEntitySubType.NULL_SUBTYPE, - catalogEntity, - CatalogRoleEntity::of) + PolarisEntitySubType.NULL_SUBTYPE) + .stream() + .map(CatalogRoleEntity::of) .map(CatalogRoleEntity::asCatalogRole) .toList(); } diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 8102a8030..4c0f7af2e 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -1124,8 +1124,8 @@ public class IcebergCatalog extends BaseMetastoreViewCatalog ListEntitiesResult siblingNamespacesResult = getMetaStoreManager() .listEntities( - callContext.getPolarisCallContext(), - parentPath.stream().map(PolarisEntity::toCore).collect(Collectors.toList()), + getCurrentPolarisContext(), + PolarisEntity.toCoreList(parentPath), PolarisEntityType.NAMESPACE, PolarisEntitySubType.ANY_SUBTYPE, PageToken.readEverything()); @@ -1141,10 +1141,8 @@ public class IcebergCatalog extends BaseMetastoreViewCatalog ListEntitiesResult siblingTablesResult = getMetaStoreManager() .listEntities( - callContext.getPolarisCallContext(), - parentPath.stream() - .map(PolarisEntity::toCore) - .collect(Collectors.toList()), + getCurrentPolarisContext(), + PolarisEntity.toCoreList(parentPath), PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE, PageToken.readEverything()); diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java index e0edebfc6..6cc8ba35d 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java @@ -50,7 +50,6 @@ import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; import org.apache.polaris.core.persistence.PolicyMappingAlreadyExistsException; import org.apache.polaris.core.persistence.dao.entity.EntityResult; import org.apache.polaris.core.persistence.dao.entity.LoadPolicyMappingsResult; -import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; import org.apache.polaris.core.policy.PolicyEntity; import org.apache.polaris.core.policy.PolicyType; @@ -162,34 +161,16 @@ public class PolicyCatalog { } List<PolarisEntity> catalogPath = resolvedEntities.getRawFullPath(); - List<PolicyEntity> policyEntities = - metaStoreManager - .listEntities( - callContext.getPolarisCallContext(), - PolarisEntity.toCoreList(catalogPath), - PolarisEntityType.POLICY, - PolarisEntitySubType.NULL_SUBTYPE, - PageToken.readEverything()) - .getEntities() - .stream() - .map( - polarisEntityActiveRecord -> - PolicyEntity.of( - metaStoreManager - .loadEntity( - callContext.getPolarisCallContext(), - polarisEntityActiveRecord.getCatalogId(), - polarisEntityActiveRecord.getId(), - polarisEntityActiveRecord.getType()) - .getEntity())) - .filter( - policyEntity -> policyType == null || policyEntity.getPolicyType() == policyType) - .toList(); - - List<PolarisEntity.NameAndId> entities = - policyEntities.stream().map(PolarisEntity::nameAndId).toList(); - - return entities.stream() + // TODO: when the "policyType" filter is null we should only call "listEntities" instead + return metaStoreManager + .loadEntitiesAll( + callContext.getPolarisCallContext(), + PolarisEntity.toCoreList(catalogPath), + PolarisEntityType.POLICY, + PolarisEntitySubType.NULL_SUBTYPE) + .stream() + .map(PolicyEntity::of) + .filter(policyEntity -> policyType == null || policyEntity.getPolicyType() == policyType) .map( entity -> PolicyIdentifier.builder() diff --git a/runtime/service/src/test/java/org/apache/polaris/service/admin/ManagementServiceTest.java b/runtime/service/src/test/java/org/apache/polaris/service/admin/ManagementServiceTest.java index 422af05cf..d5a8b9c86 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/admin/ManagementServiceTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/admin/ManagementServiceTest.java @@ -49,7 +49,6 @@ import org.apache.polaris.core.entity.PrincipalEntity; import org.apache.polaris.core.entity.PrincipalRoleEntity; import org.apache.polaris.core.persistence.MetaStoreManagerFactory; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; -import org.apache.polaris.core.persistence.dao.entity.BaseResult; import org.apache.polaris.core.persistence.dao.entity.CreateCatalogResult; import org.apache.polaris.core.persistence.dao.entity.EntityResult; import org.apache.polaris.core.secrets.UnsafeInMemorySecretsManager; @@ -57,7 +56,6 @@ import org.apache.polaris.service.TestServices; import org.apache.polaris.service.config.ReservedProperties; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.Mockito; public class ManagementServiceTest { private TestServices services; @@ -248,10 +246,9 @@ public class ManagementServiceTest { .isInstanceOf(ValidationException.class); } - /** Simulates the case when a catalog is dropped after being found while listing all catalogs. */ @Test - public void testCatalogNotReturnedWhenDeletedAfterListBeforeGet() { - PolarisMetaStoreManager metaStoreManager = Mockito.spy(setupMetaStoreManager()); + public void testCanListCatalogs() { + PolarisMetaStoreManager metaStoreManager = setupMetaStoreManager(); PolarisCallContext callContext = services.newCallContext(); PolarisAdminService polarisAdminService = setupPolarisAdminService(metaStoreManager, callContext); @@ -267,6 +264,8 @@ public class ManagementServiceTest { PolarisEntityConstants.getRootEntityId(), "my-catalog-1"), List.of()); + assertThat(catalog1.isSuccess()).isTrue(); + CreateCatalogResult catalog2 = metaStoreManager.createCatalog( callContext, @@ -278,20 +277,12 @@ public class ManagementServiceTest { PolarisEntityConstants.getRootEntityId(), "my-catalog-2"), List.of()); - - Mockito.doAnswer( - invocation -> { - Object entityId = invocation.getArgument(2); - if (entityId.equals(catalog1.getCatalog().getId())) { - return new EntityResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, ""); - } - return invocation.callRealMethod(); - }) - .when(metaStoreManager) - .loadEntity(Mockito.any(), Mockito.anyLong(), Mockito.anyLong(), Mockito.any()); + assertThat(catalog2.isSuccess()).isTrue(); List<Catalog> catalogs = polarisAdminService.listCatalogs(); - assertThat(catalogs.size()).isEqualTo(1); - assertThat(catalogs.getFirst().getName()).isEqualTo(catalog2.getCatalog().getName()); + assertThat(catalogs.size()).isEqualTo(2); + assertThat(catalogs) + .extracting(Catalog::getName) + .containsExactlyInAnyOrder("my-catalog-1", "my-catalog-2"); } }