From b3a06bbc89f26c3904591d9064118fbe5a764dcf Mon Sep 17 00:00:00 2001 From: Christopher Lambert <1204398+XN137@users.noreply.github.com> Date: Tue, 5 Aug 2025 09:50:39 +0200 Subject: [PATCH] Remove overloads of BasePersistence.listEntities `BasePersistence.listEntities` and `TransactionalPersistence.listEntitiesInCurrentTxn` are afaict only overloaded to make the `entityFilter` and `transformer` parameters "optional" for some callers. the most central method still has to have support for both of them. using default methods in the interface would enable this more clearly but as it turns out, hardly any callers are utilizing those overloads and thus if we simply remove the overloads we have less code and things become clearer across the board. we also add a `EntityNameLookupRecord.fromEntity` factory method to replace the overloaded constructor. --- ...olarisEclipseLinkMetaStoreSessionImpl.java | 39 --------------- .../eclipselink/PolarisEclipseLinkStore.java | 3 +- .../jdbc/JdbcBasePersistenceImpl.java | 40 +-------------- .../core/entity/EntityNameLookupRecord.java | 18 +++---- .../AtomicOperationMetaStoreManager.java | 9 +++- .../core/persistence/BasePersistence.java | 41 +--------------- .../AbstractTransactionalPersistence.java | 31 ------------ .../TransactionalMetaStoreManagerImpl.java | 1 + .../TransactionalPersistence.java | 19 ------- .../TreeMapTransactionalPersistenceImpl.java | 49 +------------------ .../BasePolarisMetaStoreManagerTest.java | 15 +----- 11 files changed, 24 insertions(+), 241 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 cf2e9053b1..39b6940047 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 @@ -19,7 +19,6 @@ package org.apache.polaris.extension.persistence.impl.eclipselink; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Predicates; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import jakarta.persistence.EntityManager; @@ -424,44 +423,6 @@ public List lookupEntityActiveBatchInCurrentTxn( .collect(Collectors.toList()); } - /** {@inheritDoc} */ - @Override - public @Nonnull Page listEntitiesInCurrentTxn( - @Nonnull PolarisCallContext callCtx, - long catalogId, - long parentId, - @Nonnull PolarisEntityType entityType, - @Nonnull PageToken pageToken) { - return this.listEntitiesInCurrentTxn( - callCtx, catalogId, parentId, entityType, Predicates.alwaysTrue(), pageToken); - } - - @Override - public @Nonnull Page listEntitiesInCurrentTxn( - @Nonnull PolarisCallContext callCtx, - long catalogId, - long parentId, - @Nonnull PolarisEntityType entityType, - @Nonnull Predicate entityFilter, - @Nonnull PageToken pageToken) { - // full range scan under the parent for that type - return this.listEntitiesInCurrentTxn( - callCtx, - catalogId, - parentId, - entityType, - entityFilter, - entity -> - new EntityNameLookupRecord( - entity.getCatalogId(), - entity.getId(), - entity.getParentId(), - entity.getName(), - entity.getTypeCode(), - entity.getSubTypeCode()), - pageToken); - } - @Override public @Nonnull Page listEntitiesInCurrentTxn( @Nonnull PolarisCallContext callCtx, diff --git a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java index 60251a7b65..9480ed2337 100644 --- a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java +++ b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java @@ -105,7 +105,8 @@ void writeToEntitiesActive(EntityManager session, PolarisBaseEntity entity) { ModelEntityActive model = lookupEntityActive(session, new PolarisEntitiesActiveKey(entity)); if (model == null) { - session.persist(ModelEntityActive.fromEntityActive(new EntityNameLookupRecord(entity))); + session.persist( + ModelEntityActive.fromEntityActive(EntityNameLookupRecord.fromEntity(entity))); } } 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 76da5fa92a..b6c4ebac24 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 @@ -37,7 +37,6 @@ import java.util.function.Predicate; import java.util.stream.Collectors; import org.apache.polaris.core.PolarisCallContext; -import org.apache.polaris.core.entity.EntityNameLookupRecord; import org.apache.polaris.core.entity.LocationBasedEntity; import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.entity.PolarisChangeTrackingVersions; @@ -420,50 +419,13 @@ public List lookupEntityVersions( .collect(Collectors.toList()); } - @Nonnull - @Override - public Page listEntities( - @Nonnull PolarisCallContext callCtx, - long catalogId, - long parentId, - @Nonnull PolarisEntityType entityType, - @Nonnull PageToken pageToken) { - return listEntities( - callCtx, - catalogId, - parentId, - entityType, - entity -> true, - EntityNameLookupRecord::new, - pageToken); - } - - @Nonnull - @Override - public Page listEntities( - @Nonnull PolarisCallContext callCtx, - long catalogId, - long parentId, - @Nonnull PolarisEntityType entityType, - @Nonnull Predicate entityFilter, - @Nonnull PageToken pageToken) { - return listEntities( - callCtx, - catalogId, - parentId, - entityType, - entityFilter, - EntityNameLookupRecord::new, - pageToken); - } - @Nonnull @Override public Page listEntities( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, - PolarisEntityType entityType, + @Nonnull PolarisEntityType entityType, @Nonnull Predicate entityFilter, @Nonnull Function transformer, @Nonnull PageToken pageToken) { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/EntityNameLookupRecord.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/EntityNameLookupRecord.java index 6d150a50f0..c4f33fb097 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/EntityNameLookupRecord.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/EntityNameLookupRecord.java @@ -89,14 +89,14 @@ public EntityNameLookupRecord( this.subTypeCode = subTypeCode; } - /** Constructor to create the object with provided entity */ - public EntityNameLookupRecord(PolarisBaseEntity entity) { - this.catalogId = entity.getCatalogId(); - this.id = entity.getId(); - this.parentId = entity.getParentId(); - this.typeCode = entity.getTypeCode(); - this.name = entity.getName(); - this.subTypeCode = entity.getSubTypeCode(); + public static EntityNameLookupRecord fromEntity(PolarisBaseEntity entity) { + return new EntityNameLookupRecord( + entity.getCatalogId(), + entity.getId(), + entity.getParentId(), + entity.getName(), + entity.getTypeCode(), + entity.getSubTypeCode()); } @Override @@ -119,7 +119,7 @@ public int hashCode() { @Override public String toString() { - return "PolarisEntitiesActiveRecord{" + return "EntityNameLookupRecord{" + "catalogId=" + catalogId + ", id=" 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 1d1bf5773a..28954b1a45 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 @@ -709,7 +709,14 @@ private void revokeGrantRecord( : entity -> true; Page resultPage = - ms.listEntities(callCtx, catalogId, parentId, entityType, filter, pageToken); + ms.listEntities( + callCtx, + catalogId, + parentId, + entityType, + filter, + EntityNameLookupRecord::fromEntity, + pageToken); // TODO: Use post-validation to enforce consistent view against catalogPath. In the // meantime, happens-before ordering semantics aren't guaranteed during high-concurrency 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 26ccd8de39..1a5f0f2aa8 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 @@ -241,7 +241,7 @@ default EntityNameLookupRecord lookupEntityIdAndSubTypeByName( if (baseEntity == null) { return null; } - return new EntityNameLookupRecord(baseEntity); + return EntityNameLookupRecord.fromEntity(baseEntity); } /** @@ -268,45 +268,6 @@ List lookupEntities( List lookupEntityVersions( @Nonnull PolarisCallContext callCtx, List entityIds); - /** - * List all entities of the specified type which are child entities of the specified parent - * - * @param callCtx call context - * @param catalogId catalog id for that entity, NULL_ID if the entity is top-level - * @param parentId id of the parent, can be the special 0 value representing the root entity - * @param entityType type of entities to list - * @param pageToken the token to start listing after - * @return the list of entities for the specified list operation - */ - @Nonnull - Page listEntities( - @Nonnull PolarisCallContext callCtx, - long catalogId, - long parentId, - @Nonnull PolarisEntityType entityType, - @Nonnull PageToken pageToken); - - /** - * List entities where some predicate returns true - * - * @param callCtx call context - * @param catalogId catalog id for that entity, NULL_ID if the entity is top-level - * @param parentId id of the parent, can be the special 0 value representing the root entity - * @param entityType type of entities to list - * @param entityFilter the filter to be applied to each entity. Only entities where the predicate - * returns true are returned in the list - * @param pageToken the token to start listing after - * @return the list of entities for which the predicate returns true - */ - @Nonnull - Page listEntities( - @Nonnull PolarisCallContext callCtx, - long catalogId, - long parentId, - @Nonnull PolarisEntityType entityType, - @Nonnull Predicate entityFilter, - @Nonnull PageToken pageToken); - /** * List entities where some predicate returns true and transform the entities with a function * 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 f08b85e122..aa57177ecf 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 @@ -351,37 +351,6 @@ public List lookupEntityVersions( callCtx, () -> this.lookupEntityVersionsInCurrentTxn(callCtx, entityIds)); } - /** {@inheritDoc} */ - @Override - @Nonnull - public Page listEntities( - @Nonnull PolarisCallContext callCtx, - long catalogId, - long parentId, - @Nonnull PolarisEntityType entityType, - @Nonnull PageToken pageToken) { - return runInReadTransaction( - callCtx, - () -> this.listEntitiesInCurrentTxn(callCtx, catalogId, parentId, entityType, pageToken)); - } - - /** {@inheritDoc} */ - @Override - @Nonnull - public Page listEntities( - @Nonnull PolarisCallContext callCtx, - long catalogId, - long parentId, - @Nonnull PolarisEntityType entityType, - @Nonnull Predicate entityFilter, - @Nonnull PageToken pageToken) { - return runInReadTransaction( - callCtx, - () -> - this.listEntitiesInCurrentTxn( - callCtx, catalogId, parentId, entityType, entityFilter, pageToken)); - } - /** {@inheritDoc} */ @Override @Nonnull 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 2a4444b44d..7da5e83140 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 @@ -717,6 +717,7 @@ private void bootstrapPolarisService( resolver.getParentId(), entityType, filter, + EntityNameLookupRecord::fromEntity, pageToken); // done 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 1c58334d55..e2406479ca 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 @@ -201,25 +201,6 @@ List lookupEntitiesInCurrentTxn( List lookupEntityVersionsInCurrentTxn( @Nonnull PolarisCallContext callCtx, List entityIds); - /** See {@link org.apache.polaris.core.persistence.BasePersistence#listEntities} */ - @Nonnull - Page listEntitiesInCurrentTxn( - @Nonnull PolarisCallContext callCtx, - long catalogId, - long parentId, - @Nonnull PolarisEntityType entityType, - @Nonnull PageToken pageToken); - - /** See {@link org.apache.polaris.core.persistence.BasePersistence#listEntities} */ - @Nonnull - Page listEntitiesInCurrentTxn( - @Nonnull PolarisCallContext callCtx, - long catalogId, - long parentId, - @Nonnull PolarisEntityType entityType, - @Nonnull Predicate entityFilter, - @Nonnull PageToken pageToken); - /** See {@link org.apache.polaris.core.persistence.BasePersistence#listEntities} */ @Nonnull Page listEntitiesInCurrentTxn( 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 6ebb18e8ce..956c777f14 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 @@ -18,7 +18,6 @@ */ package org.apache.polaris.core.persistence.transactional; -import com.google.common.base.Predicates; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import java.util.Comparator; @@ -288,15 +287,7 @@ public EntityNameLookupRecord lookupEntityActiveInCurrentTxn( 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 : EntityNameLookupRecord.fromEntity(entity); } /** {@inheritDoc} */ @@ -311,44 +302,6 @@ public List lookupEntityActiveBatchInCurrentTxn( .collect(Collectors.toList()); } - /** {@inheritDoc} */ - @Override - public @Nonnull Page listEntitiesInCurrentTxn( - @Nonnull PolarisCallContext callCtx, - long catalogId, - long parentId, - @Nonnull PolarisEntityType entityType, - @Nonnull PageToken pageToken) { - return this.listEntitiesInCurrentTxn( - callCtx, catalogId, parentId, entityType, Predicates.alwaysTrue(), pageToken); - } - - @Override - public @Nonnull Page listEntitiesInCurrentTxn( - @Nonnull PolarisCallContext callCtx, - long catalogId, - long parentId, - @Nonnull PolarisEntityType entityType, - @Nonnull Predicate entityFilter, - @Nonnull PageToken pageToken) { - // full range scan under the parent for that type - return this.listEntitiesInCurrentTxn( - callCtx, - catalogId, - parentId, - entityType, - entityFilter, - entity -> - new EntityNameLookupRecord( - entity.getCatalogId(), - entity.getId(), - entity.getParentId(), - entity.getName(), - entity.getTypeCode(), - entity.getSubTypeCode()), - pageToken); - } - @Override public @Nonnull Page listEntitiesInCurrentTxn( @Nonnull PolarisCallContext callCtx, 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 1b57ad1b2e..8b3d5371c3 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 @@ -130,20 +130,7 @@ protected void testCreateEntities() { .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())); + EntityNameLookupRecord.fromEntity(task1), EntityNameLookupRecord.fromEntity(task2)); } @Test