From c67fc4cabba8f6b77d90551177b8ce3c94926a2d Mon Sep 17 00:00:00 2001 From: Honah J Date: Tue, 20 May 2025 14:40:07 -0500 Subject: [PATCH 1/3] Add policyTypeCode to slice/index for future optimization --- ...olarisEclipseLinkMetaStoreSessionImpl.java | 11 +++-- .../eclipselink/PolarisEclipseLinkStore.java | 24 ++++++++--- .../jpa/models/ModelPolicyMappingRecord.java | 2 +- .../jdbc/JdbcBasePersistenceImpl.java | 17 ++++++-- .../relational/jdbc/QueryGenerator.java | 33 ++++++++------- .../AtomicOperationMetaStoreManager.java | 11 ++++- .../AbstractTransactionalPersistence.java | 12 ++++-- .../TransactionalMetaStoreManagerImpl.java | 12 ++++-- .../transactional/TreeMapMetaStore.java | 3 +- .../TreeMapTransactionalPersistenceImpl.java | 42 ++++++++++++------- .../core/policy/PolicyMappingPersistence.java | 10 +++-- ...TransactionalPolicyMappingPersistence.java | 9 ++-- .../PolarisTestMetaStoreManager.java | 3 +- 13 files changed, 130 insertions(+), 59 deletions(-) diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index b6bd237623..41b77c942f 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -703,7 +703,7 @@ public void deleteFromPolicyMappingRecordsInCurrentTxn( @Override public void deleteAllEntityPolicyMappingRecordsInCurrentTxn( @Nonnull PolarisCallContext callCtx, - @Nonnull PolarisEntityCore entity, + @Nonnull PolarisBaseEntity entity, @Nonnull List mappingOnTarget, @Nonnull List mappingOnPolicy) { this.store.deleteAllEntityPolicyMappingRecords(localSession.get(), entity); @@ -760,8 +760,13 @@ public List loadAllPoliciesOnTargetInCurrentTxn( @Nonnull @Override public List loadAllTargetsOnPolicyInCurrentTxn( - @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { - return this.store.loadAllTargetsOnPolicy(localSession.get(), policyCatalogId, policyId).stream() + @Nonnull PolarisCallContext callCtx, + long policyCatalogId, + long policyId, + long policyTypeCode) { + return this.store + .loadAllTargetsOnPolicy(localSession.get(), policyCatalogId, policyId, policyTypeCode) + .stream() .map(ModelPolicyMappingRecord::toPolicyMappingRecord) .toList(); } diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java index 4e992e07f4..a4ef898fd5 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java @@ -37,6 +37,7 @@ import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; +import org.apache.polaris.core.policy.PolicyEntity; import org.apache.polaris.jpa.models.ModelEntity; import org.apache.polaris.jpa.models.ModelEntityActive; import org.apache.polaris.jpa.models.ModelEntityChangeTracking; @@ -461,13 +462,22 @@ void deleteFromPolicyMappingRecords( session.remove(lookupPolicyMappingRecord); } - void deleteAllEntityPolicyMappingRecords(EntityManager session, PolarisEntityCore entity) { + void deleteAllEntityPolicyMappingRecords(EntityManager session, PolarisBaseEntity entity) { diagnosticServices.check(session != null, "session_is_null"); checkInitialized(); - loadAllTargetsOnPolicy(session, entity.getCatalogId(), entity.getId()).forEach(session::remove); - loadAllPoliciesOnTarget(session, entity.getCatalogId(), entity.getId()) - .forEach(session::remove); + if (entity.getType() == PolarisEntityType.POLICY) { + PolicyEntity policyEntity = PolicyEntity.of(entity); + loadAllTargetsOnPolicy( + session, + policyEntity.getCatalogId(), + policyEntity.getId(), + policyEntity.getPolicyTypeCode()) + .forEach(session::remove); + } else { + loadAllPoliciesOnTarget(session, entity.getCatalogId(), entity.getId()) + .forEach(session::remove); + } } ModelPolicyMappingRecord lookupPolicyMappingRecord( @@ -534,16 +544,18 @@ List loadAllPoliciesOnTarget( } List loadAllTargetsOnPolicy( - EntityManager session, long policyCatalogId, long policyId) { + EntityManager session, long policyCatalogId, long policyId, long policyTypeCode) { diagnosticServices.check(session != null, "session_is_null"); checkInitialized(); return session .createQuery( "SELECT m from ModelPolicyMappingRecord m " - + "where m.policyCatalogId=:policyCatalogId " + + "where m.policyTypeCode=:policyTypeCode " + + "and m.policyCatalogId=:policyCatalogId " + "and m.policyId=:policyId", ModelPolicyMappingRecord.class) + .setParameter("policyTypeCode", policyTypeCode) .setParameter("policyCatalogId", policyCatalogId) .setParameter("policyId", policyId) .getResultList(); diff --git a/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/ModelPolicyMappingRecord.java b/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/ModelPolicyMappingRecord.java index 0448dec676..0a6f587c4d 100644 --- a/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/ModelPolicyMappingRecord.java +++ b/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/ModelPolicyMappingRecord.java @@ -33,7 +33,7 @@ indexes = { @Index( name = "POLICY_MAPPING_RECORDS_BY_POLICY_INDEX", - columnList = "policyCatalogId,policyId,targetCatalogId,targetId") + columnList = "policyTypeCode,policyCatalogId,policyId,targetCatalogId,targetId") }) @PrimaryKey( columns = { diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index e229100fe2..d859c3cb8d 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -780,7 +780,7 @@ public void deleteFromPolicyMappingRecords( @Override public void deleteAllEntityPolicyMappingRecords( @Nonnull PolarisCallContext callCtx, - @Nonnull PolarisEntityCore entity, + @Nonnull PolarisBaseEntity entity, @Nonnull List mappingOnTarget, @Nonnull List mappingOnPolicy) { try { @@ -855,9 +855,20 @@ public List loadAllPoliciesOnTarget( @Nonnull @Override public List loadAllTargetsOnPolicy( - @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { + @Nonnull PolarisCallContext callCtx, + long policyCatalogId, + long policyId, + long policyTypeCode) { Map params = - Map.of("policy_catalog_id", policyCatalogId, "policy_id", policyId, "realm_id", realmId); + Map.of( + "policy_type_code", + policyTypeCode, + "policy_catalog_id", + policyCatalogId, + "policy_id", + policyId, + "realm_id", + realmId); String query = generateSelectQuery(new ModelPolicyMappingRecord(), params); return fetchPolicyMappingRecords(query); } diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/QueryGenerator.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/QueryGenerator.java index d8f8d0864f..5d277fd986 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/QueryGenerator.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/QueryGenerator.java @@ -21,10 +21,14 @@ import com.google.common.annotations.VisibleForTesting; import jakarta.annotation.Nonnull; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; +import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.entity.PolarisEntityCore; import org.apache.polaris.core.entity.PolarisEntityId; +import org.apache.polaris.core.entity.PolarisEntityType; +import org.apache.polaris.core.policy.PolicyEntity; import org.apache.polaris.extension.persistence.relational.jdbc.models.Converter; import org.apache.polaris.extension.persistence.relational.jdbc.models.ModelEntity; import org.apache.polaris.extension.persistence.relational.jdbc.models.ModelGrantRecord; @@ -60,23 +64,20 @@ public static String generateDeleteQueryForEntityGrantRecords( } public static String generateDeleteQueryForEntityPolicyMappingRecords( - @Nonnull PolarisEntityCore entity, @Nonnull String realmId) { - String targetCondition = - String.format( - "target_id = %s AND target_catalog_id = %s", entity.getId(), entity.getCatalogId()); - String sourceCondition = - String.format( - "policy_id = %s AND policy_catalog_id = %s", entity.getId(), entity.getCatalogId()); + @Nonnull PolarisBaseEntity entity, @Nonnull String realmId) { + Map objMap = new HashMap<>(); + if (entity.getType() == PolarisEntityType.POLICY) { + PolicyEntity policyEntity = PolicyEntity.of(entity); + objMap.put("policy_type_code", policyEntity.getPolicyTypeCode()); + objMap.put("policy_catalog_id", policyEntity.getCatalogId()); + objMap.put("policy_id", policyEntity.getId()); + } else { + objMap.put("target_catalog_id", entity.getCatalogId()); + objMap.put("target_id", entity.getId()); + } + objMap.put("realm_id", realmId); - String whereClause = - " WHERE (" - + targetCondition - + " OR " - + sourceCondition - + ") AND realm_id = '" - + realmId - + "'"; - return generateDeleteQuery(ModelPolicyMappingRecord.class, whereClause); + return generateDeleteQuery(ModelPolicyMappingRecord.class, objMap); } public static String generateSelectQueryWithEntityIds( 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 2a32fb6f94..92ccc6eed5 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 @@ -203,7 +203,11 @@ private void dropEntity( try { final List mappingOnPolicy = (entity.getType() == PolarisEntityType.POLICY) - ? ms.loadAllTargetsOnPolicy(callCtx, entity.getCatalogId(), entity.getId()) + ? ms.loadAllTargetsOnPolicy( + callCtx, + entity.getCatalogId(), + entity.getId(), + PolicyEntity.of(entity).getPolicyTypeCode()) : List.of(); final List mappingOnTarget = (entity.getType() == PolarisEntityType.POLICY) @@ -1209,7 +1213,10 @@ private void revokeGrantRecord( // need to check if the policy is attached to any entity List records = ms.loadAllTargetsOnPolicy( - callCtx, refreshEntityToDrop.getCatalogId(), refreshEntityToDrop.getId()); + callCtx, + refreshEntityToDrop.getCatalogId(), + refreshEntityToDrop.getId(), + PolicyEntity.of(refreshEntityToDrop).getPolicyTypeCode()); if (!records.isEmpty()) { return new DropEntityResult(BaseResult.ReturnStatus.POLICY_HAS_MAPPINGS, null); } 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 e63ea6fedb..f1ef4ed92b 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 @@ -722,7 +722,7 @@ public void deleteFromPolicyMappingRecords( @Override public void deleteAllEntityPolicyMappingRecords( @Nonnull PolarisCallContext callCtx, - @Nonnull PolarisEntityCore entity, + @Nonnull PolarisBaseEntity entity, @Nonnull List mappingOnTarget, @Nonnull List mappingOnPolicy) { this.runActionInTransaction( @@ -778,8 +778,14 @@ public List loadAllPoliciesOnTarget( @Override @Nonnull public List loadAllTargetsOnPolicy( - @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { + @Nonnull PolarisCallContext callCtx, + long policyCatalogId, + long policyId, + long policyTypeCode) { return this.runInReadTransaction( - callCtx, () -> this.loadAllTargetsOnPolicyInCurrentTxn(callCtx, policyCatalogId, policyId)); + callCtx, + () -> + this.loadAllTargetsOnPolicyInCurrentTxn( + callCtx, policyCatalogId, policyId, policyTypeCode)); } } 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 62f526a6db..0224dec493 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 @@ -205,7 +205,10 @@ private void dropEntity( final List mappingOnPolicy = (entity.getType() == PolarisEntityType.POLICY) ? ms.loadAllTargetsOnPolicyInCurrentTxn( - callCtx, entity.getCatalogId(), entity.getId()) + callCtx, + entity.getCatalogId(), + entity.getId(), + PolicyEntity.of(entity).getPolicyTypeCode()) : List.of(); final List mappingOnTarget = (entity.getType() == PolarisEntityType.POLICY) @@ -1397,7 +1400,10 @@ private void bootstrapPolarisService( try { List records = ms.loadAllTargetsOnPolicyInCurrentTxn( - callCtx, refreshEntityToDrop.getCatalogId(), refreshEntityToDrop.getId()); + callCtx, + refreshEntityToDrop.getCatalogId(), + refreshEntityToDrop.getId(), + PolicyEntity.of(refreshEntityToDrop).getPolicyTypeCode()); if (!records.isEmpty()) { return new DropEntityResult(BaseResult.ReturnStatus.POLICY_HAS_MAPPINGS, null); } @@ -2448,7 +2454,7 @@ private LoadPolicyMappingsResult doLoadPoliciesOnEntity( } /** See {@link #loadPoliciesOnEntityByType(PolarisCallContext, PolarisEntityCore, PolicyType)} */ - public LoadPolicyMappingsResult doLoadPoliciesOnEntityByType( + private LoadPolicyMappingsResult doLoadPoliciesOnEntityByType( @Nonnull PolarisCallContext callCtx, @Nonnull TransactionalPersistence ms, @Nonnull PolarisEntityCore target, diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapMetaStore.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapMetaStore.java index c914140407..781732e42a 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapMetaStore.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapMetaStore.java @@ -287,7 +287,8 @@ public TreeMapMetaStore(@Nonnull PolarisDiagnostics diagnostics) { new Slice<>( policyMappingRecord -> String.format( - "%d::%d::%d::%d", + "%d::%d::%d::%d::%d", + policyMappingRecord.getPolicyTypeCode(), policyMappingRecord.getPolicyCatalogId(), policyMappingRecord.getPolicyId(), policyMappingRecord.getTargetCatalogId(), 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 304ac0ce97..fd40f56443 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 @@ -43,6 +43,7 @@ import org.apache.polaris.core.persistence.pagination.Page; import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; +import org.apache.polaris.core.policy.PolicyEntity; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider; @@ -588,20 +589,30 @@ public void deleteFromPolicyMappingRecordsInCurrentTxn( @Override public void deleteAllEntityPolicyMappingRecordsInCurrentTxn( @Nonnull PolarisCallContext callCtx, - @Nonnull PolarisEntityCore entity, + @Nonnull PolarisBaseEntity entity, @Nonnull List mappingOnTarget, @Nonnull List mappingOnPolicy) { - // build composite prefix key and delete policy mapping records on the indexed side of each - // mapping table - String prefix = this.store.buildPrefixKeyComposite(entity.getCatalogId(), entity.getId()); - this.store.getSlicePolicyMappingRecords().deleteRange(prefix); - this.store.getSlicePolicyMappingRecordsByPolicy().deleteRange(prefix); - - // also delete the other side. We need to delete these mapping one at a time versus doing a - // range delete - mappingOnTarget.forEach( - record -> this.store.getSlicePolicyMappingRecordsByPolicy().delete(record)); - mappingOnPolicy.forEach(record -> this.store.getSlicePolicyMappingRecords().delete(record)); + if (entity.getType() == PolarisEntityType.POLICY) { + PolicyEntity policyEntity = PolicyEntity.of(entity); + this.store + .getSlicePolicyMappingRecordsByPolicy() + .deleteRange( + this.store.buildPrefixKeyComposite( + policyEntity.getPolicyTypeCode(), + policyEntity.getCatalogId(), + policyEntity.getId())); + // also delete the other side. We need to delete these mapping one at a time versus doing a + // range delete + mappingOnPolicy.forEach(record -> this.store.getSlicePolicyMappingRecords().delete(record)); + } else { + this.store + .getSlicePolicyMappingRecords() + .deleteRange(this.store.buildPrefixKeyComposite(entity.getCatalogId(), entity.getId())); + // also delete the other side. We need to delete these mapping one at a time versus doing a + // range delete + mappingOnTarget.forEach( + record -> this.store.getSlicePolicyMappingRecordsByPolicy().delete(record)); + } } /** {@inheritDoc} */ @@ -644,9 +655,12 @@ record -> this.store.getSlicePolicyMappingRecordsByPolicy().delete(record)); /** {@inheritDoc} */ @Override public @Nonnull List loadAllTargetsOnPolicyInCurrentTxn( - @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { + @Nonnull PolarisCallContext callCtx, + long policyCatalogId, + long policyId, + long policyTypeCode) { return this.store .getSlicePolicyMappingRecordsByPolicy() - .readRange(this.store.buildPrefixKeyComposite(policyCatalogId, policyId)); + .readRange(this.store.buildPrefixKeyComposite(policyTypeCode, policyCatalogId, policyId)); } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java index 33a7546689..bd5f430d42 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java @@ -22,7 +22,7 @@ import jakarta.annotation.Nullable; import java.util.List; import org.apache.polaris.core.PolarisCallContext; -import org.apache.polaris.core.entity.PolarisEntityCore; +import org.apache.polaris.core.entity.PolarisBaseEntity; /** * Interface for interacting with the Polaris persistence backend for Policy Mapping operations. @@ -73,7 +73,7 @@ default void deleteFromPolicyMappingRecords( */ default void deleteAllEntityPolicyMappingRecords( @Nonnull PolarisCallContext callCtx, - @Nonnull PolarisEntityCore entity, + @Nonnull PolarisBaseEntity entity, @Nonnull List mappingOnTarget, @Nonnull List mappingOnPolicy) { throw new UnsupportedOperationException("Not Implemented"); @@ -141,11 +141,15 @@ default List loadAllPoliciesOnTarget( * @param callCtx call context * @param policyCatalogId catalog id of the policy entity, NULL_ID if the entity is top-level * @param policyId id of the policy entity + * @param policyTypeCode type code of the policy entity * @return the list of policy mapping records for the specified policy entity */ @Nonnull default List loadAllTargetsOnPolicy( - @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { + @Nonnull PolarisCallContext callCtx, + long policyCatalogId, + long policyId, + long policyTypeCode) { throw new UnsupportedOperationException("Not Implemented"); } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java index 6ad1ac80c8..344bfe9242 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java @@ -22,7 +22,7 @@ import jakarta.annotation.Nullable; import java.util.List; import org.apache.polaris.core.PolarisCallContext; -import org.apache.polaris.core.entity.PolarisEntityCore; +import org.apache.polaris.core.entity.PolarisBaseEntity; public interface TransactionalPolicyMappingPersistence { /** See {@link PolicyMappingPersistence#writeToPolicyMappingRecords} */ @@ -54,7 +54,7 @@ default void deleteFromPolicyMappingRecordsInCurrentTxn( /** See {@link PolicyMappingPersistence#deleteAllEntityPolicyMappingRecords} */ default void deleteAllEntityPolicyMappingRecordsInCurrentTxn( @Nonnull PolarisCallContext callCtx, - @Nonnull PolarisEntityCore entity, + @Nonnull PolarisBaseEntity entity, @Nonnull List mappingOnTarget, @Nonnull List mappingOnPolicy) { throw new UnsupportedOperationException("Not Implemented"); @@ -92,7 +92,10 @@ default List loadAllPoliciesOnTargetInCurrentTxn( /** See {@link PolicyMappingPersistence#loadAllTargetsOnPolicy} */ @Nonnull default List loadAllTargetsOnPolicyInCurrentTxn( - @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { + @Nonnull PolarisCallContext callCtx, + long policyCatalogId, + long policyId, + long policyTypeCode) { throw new UnsupportedOperationException("Not Implemented"); } } diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java index 8cddecae3e..7d90ab23da 100644 --- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java +++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java @@ -2888,7 +2888,8 @@ void testPolicyMappingCleanup() { BasePersistence ms = polarisCallContext.getMetaStore(); Assertions.assertThat( - ms.loadAllTargetsOnPolicy(polarisCallContext, N1_P1.getCatalogId(), N1_P1.getId())) + ms.loadAllTargetsOnPolicy( + polarisCallContext, N1_P1.getCatalogId(), N1_P1.getId(), N1_P1.getPolicyTypeCode())) .isEmpty(); attachPolicyToTarget(List.of(catalog, N1, N1_N2), N1_N2_T1, List.of(catalog, N1), N1_P2); From dc12b134767deb7a342a9394321364b6b2d6e05f Mon Sep 17 00:00:00 2001 From: Honah J Date: Tue, 20 May 2025 16:59:52 -0500 Subject: [PATCH 2/3] Fix policyTypeCode type --- .../eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java | 2 +- .../persistence/impl/eclipselink/PolarisEclipseLinkStore.java | 4 ++-- .../persistence/relational/jdbc/JdbcBasePersistenceImpl.java | 2 +- .../transactional/AbstractTransactionalPersistence.java | 2 +- .../transactional/TreeMapTransactionalPersistenceImpl.java | 2 +- .../apache/polaris/core/policy/PolicyMappingPersistence.java | 2 +- .../core/policy/TransactionalPolicyMappingPersistence.java | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index 41b77c942f..151d1a5487 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -763,7 +763,7 @@ public List loadAllTargetsOnPolicyInCurrentTxn( @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId, - long policyTypeCode) { + int policyTypeCode) { return this.store .loadAllTargetsOnPolicy(localSession.get(), policyCatalogId, policyId, policyTypeCode) .stream() diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java index a4ef898fd5..16fb32356e 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java @@ -484,7 +484,7 @@ ModelPolicyMappingRecord lookupPolicyMappingRecord( EntityManager session, long targetCatalogId, long targetId, - long policyTypeCode, + int policyTypeCode, long policyCatalogId, long policyId) { diagnosticServices.check(session != null, "session_is_null"); @@ -544,7 +544,7 @@ List loadAllPoliciesOnTarget( } List loadAllTargetsOnPolicy( - EntityManager session, long policyCatalogId, long policyId, long policyTypeCode) { + EntityManager session, long policyCatalogId, long policyId, int policyTypeCode) { diagnosticServices.check(session != null, "session_is_null"); checkInitialized(); diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index d859c3cb8d..5fdb320064 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -858,7 +858,7 @@ public List loadAllTargetsOnPolicy( @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId, - long policyTypeCode) { + int policyTypeCode) { Map params = Map.of( "policy_type_code", 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 f1ef4ed92b..f08b85e122 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 @@ -781,7 +781,7 @@ public List loadAllTargetsOnPolicy( @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId, - long policyTypeCode) { + int policyTypeCode) { return this.runInReadTransaction( callCtx, () -> 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 fd40f56443..44b37c2760 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 @@ -658,7 +658,7 @@ record -> this.store.getSlicePolicyMappingRecordsByPolicy().delete(record)); @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId, - long policyTypeCode) { + int policyTypeCode) { return this.store .getSlicePolicyMappingRecordsByPolicy() .readRange(this.store.buildPrefixKeyComposite(policyTypeCode, policyCatalogId, policyId)); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java index bd5f430d42..0456f7b32f 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java @@ -149,7 +149,7 @@ default List loadAllTargetsOnPolicy( @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId, - long policyTypeCode) { + int policyTypeCode) { throw new UnsupportedOperationException("Not Implemented"); } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java index 344bfe9242..398ddb1a16 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java @@ -95,7 +95,7 @@ default List loadAllTargetsOnPolicyInCurrentTxn( @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId, - long policyTypeCode) { + int policyTypeCode) { throw new UnsupportedOperationException("Not Implemented"); } } From 437653c27031456111dd46621975d7c9e66b6636 Mon Sep 17 00:00:00 2001 From: Honah J Date: Thu, 22 May 2025 13:51:18 -0700 Subject: [PATCH 3/3] Address PR comments --- .../relational/jdbc/QueryGenerator.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/QueryGenerator.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/QueryGenerator.java index 5d277fd986..ccd457b6a8 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/QueryGenerator.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/QueryGenerator.java @@ -65,19 +65,19 @@ public static String generateDeleteQueryForEntityGrantRecords( public static String generateDeleteQueryForEntityPolicyMappingRecords( @Nonnull PolarisBaseEntity entity, @Nonnull String realmId) { - Map objMap = new HashMap<>(); + Map queryParams = new HashMap<>(); if (entity.getType() == PolarisEntityType.POLICY) { PolicyEntity policyEntity = PolicyEntity.of(entity); - objMap.put("policy_type_code", policyEntity.getPolicyTypeCode()); - objMap.put("policy_catalog_id", policyEntity.getCatalogId()); - objMap.put("policy_id", policyEntity.getId()); + queryParams.put("policy_type_code", policyEntity.getPolicyTypeCode()); + queryParams.put("policy_catalog_id", policyEntity.getCatalogId()); + queryParams.put("policy_id", policyEntity.getId()); } else { - objMap.put("target_catalog_id", entity.getCatalogId()); - objMap.put("target_id", entity.getId()); + queryParams.put("target_catalog_id", entity.getCatalogId()); + queryParams.put("target_id", entity.getId()); } - objMap.put("realm_id", realmId); + queryParams.put("realm_id", realmId); - return generateDeleteQuery(ModelPolicyMappingRecord.class, objMap); + return generateDeleteQuery(ModelPolicyMappingRecord.class, queryParams); } public static String generateSelectQueryWithEntityIds(