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");
   }
 }


Reply via email to