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 <[email protected]>
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");
}
}
