Skip to content

[Policy Store] Add policyTypeCode to Slice/Index for Future Filtering Support and Update Policy Persistence Method #1628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 22, 2025
Merged

[Policy Store] Add policyTypeCode to Slice/Index for Future Filtering Support and Update Policy Persistence Method #1628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c67fc4c
Add policyTypeCode to slice/index for future optimization
HonahX May 20, 2025
dc12b13
Fix policyTypeCode type
HonahX May 20, 2025
437653c
Address PR comments
HonahX May 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions ...olaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ public void deleteFromPolicyMappingRecordsInCurrentTxn(
@Override
public void deleteAllEntityPolicyMappingRecordsInCurrentTxn(
@Nonnull PolarisCallContext callCtx,
@Nonnull PolarisEntityCore entity,
@Nonnull PolarisBaseEntity entity,
Copy link
Contributor

@flyrain flyrain May 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker: I feel like these two classes are interchangeable in Polaris. Can we merge them to avoid any type mismatching like this? cc @dennishuo

@Nonnull List<PolarisPolicyMappingRecord> mappingOnTarget,
@Nonnull List<PolarisPolicyMappingRecord> mappingOnPolicy) {
this.store.deleteAllEntityPolicyMappingRecords(localSession.get(), entity);
Expand Down Expand Up @@ -760,8 +760,13 @@ public List<PolarisPolicyMappingRecord> loadAllPoliciesOnTargetInCurrentTxn(
@Nonnull
@Override
public List<PolarisPolicyMappingRecord> loadAllTargetsOnPolicyInCurrentTxn(
@Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) {
return this.store.loadAllTargetsOnPolicy(localSession.get(), policyCatalogId, policyId).stream()
@Nonnull PolarisCallContext callCtx,
long policyCatalogId,
long policyId,
int policyTypeCode) {
return this.store
.loadAllTargetsOnPolicy(localSession.get(), policyCatalogId, policyId, policyTypeCode)
.stream()
.map(ModelPolicyMappingRecord::toPolicyMappingRecord)
.toList();
}
Expand Down
26 changes: 19 additions & 7 deletions ...va/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -461,20 +462,29 @@ 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(
EntityManager session,
long targetCatalogId,
long targetId,
long policyTypeCode,
int policyTypeCode,
long policyCatalogId,
long policyId) {
diagnosticServices.check(session != null, "session_is_null");
Expand Down Expand Up @@ -534,16 +544,18 @@ List<ModelPolicyMappingRecord> loadAllPoliciesOnTarget(
}

List<ModelPolicyMappingRecord> loadAllTargetsOnPolicy(
EntityManager session, long policyCatalogId, long policyId) {
EntityManager session, long policyCatalogId, long policyId, int 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();
Expand Down
2 changes: 1 addition & 1 deletion ...tence/jpa-model/src/main/java/org/apache/polaris/jpa/models/ModelPolicyMappingRecord.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
indexes = {
@Index(
name = "POLICY_MAPPING_RECORDS_BY_POLICY_INDEX",
columnList = "policyCatalogId,policyId,targetCatalogId,targetId")
columnList = "policyTypeCode,policyCatalogId,policyId,targetCatalogId,targetId")
Copy link
Contributor

@dimas-b dimas-b May 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it's worth bothering with EclipseLink support for new features since EclipseLink is deprecated.

Copy link
Contributor

@flyrain flyrain May 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK with either way. Adding it to EclipseLink is nice to have.

Copy link
Contributor Author

@HonahX HonahX May 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since policy support is already available in EclipseLink and this PR introduces only a minor change ahead of the 1.0 release, I think it's reasonable to include it. My understanding is that we don’t plan to add any major new features to EclipseLink moving forward.

})
@PrimaryKey(
columns = {
Expand Down
17 changes: 14 additions & 3 deletions ...ava/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,7 @@ public void deleteFromPolicyMappingRecords(
@Override
public void deleteAllEntityPolicyMappingRecords(
@Nonnull PolarisCallContext callCtx,
@Nonnull PolarisEntityCore entity,
@Nonnull PolarisBaseEntity entity,
@Nonnull List<PolarisPolicyMappingRecord> mappingOnTarget,
@Nonnull List<PolarisPolicyMappingRecord> mappingOnPolicy) {
try {
Expand Down Expand Up @@ -855,9 +855,20 @@ public List<PolarisPolicyMappingRecord> loadAllPoliciesOnTarget(
@Nonnull
@Override
public List<PolarisPolicyMappingRecord> loadAllTargetsOnPolicy(
@Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) {
@Nonnull PolarisCallContext callCtx,
long policyCatalogId,
long policyId,
int policyTypeCode) {
Map<String, Object> 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);
}
Expand Down
33 changes: 17 additions & 16 deletions ...rc/main/java/org/apache/polaris/extension/persistence/relational/jdbc/QueryGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, Object> queryParams = new HashMap<>();
if (entity.getType() == PolarisEntityType.POLICY) {
PolicyEntity policyEntity = PolicyEntity.of(entity);
queryParams.put("policy_type_code", policyEntity.getPolicyTypeCode());
queryParams.put("policy_catalog_id", policyEntity.getCatalogId());
queryParams.put("policy_id", policyEntity.getId());
} else {
queryParams.put("target_catalog_id", entity.getCatalogId());
queryParams.put("target_id", entity.getId());
}
queryParams.put("realm_id", realmId);

String whereClause =
" WHERE ("
+ targetCondition
+ " OR "
+ sourceCondition
+ ") AND realm_id = '"
+ realmId
+ "'";
return generateDeleteQuery(ModelPolicyMappingRecord.class, whereClause);
return generateDeleteQuery(ModelPolicyMappingRecord.class, queryParams);
}

public static String generateSelectQueryWithEntityIds(
Expand Down
11 changes: 9 additions & 2 deletions ...re/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,11 @@ private void dropEntity(
try {
final List<PolarisPolicyMappingRecord> 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<PolarisPolicyMappingRecord> mappingOnTarget =
(entity.getType() == PolarisEntityType.POLICY)
Expand Down Expand Up @@ -1209,7 +1213,10 @@ private void revokeGrantRecord(
// need to check if the policy is attached to any entity
List<PolarisPolicyMappingRecord> 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);
}
Expand Down
12 changes: 9 additions & 3 deletions ...a/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@ public void deleteFromPolicyMappingRecords(
@Override
public void deleteAllEntityPolicyMappingRecords(
@Nonnull PolarisCallContext callCtx,
@Nonnull PolarisEntityCore entity,
@Nonnull PolarisBaseEntity entity,
@Nonnull List<PolarisPolicyMappingRecord> mappingOnTarget,
@Nonnull List<PolarisPolicyMappingRecord> mappingOnPolicy) {
this.runActionInTransaction(
Expand Down Expand Up @@ -778,8 +778,14 @@ public List<PolarisPolicyMappingRecord> loadAllPoliciesOnTarget(
@Override
@Nonnull
public List<PolarisPolicyMappingRecord> loadAllTargetsOnPolicy(
@Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) {
@Nonnull PolarisCallContext callCtx,
long policyCatalogId,
long policyId,
int policyTypeCode) {
return this.runInReadTransaction(
callCtx, () -> this.loadAllTargetsOnPolicyInCurrentTxn(callCtx, policyCatalogId, policyId));
callCtx,
() ->
this.loadAllTargetsOnPolicyInCurrentTxn(
callCtx, policyCatalogId, policyId, policyTypeCode));
}
}
12 changes: 9 additions & 3 deletions .../org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,10 @@ private void dropEntity(
final List<PolarisPolicyMappingRecord> 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<PolarisPolicyMappingRecord> mappingOnTarget =
(entity.getType() == PolarisEntityType.POLICY)
Expand Down Expand Up @@ -1397,7 +1400,10 @@ private void bootstrapPolarisService(
try {
List<PolarisPolicyMappingRecord> 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);
}
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion ...ore/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapMetaStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
42 changes: 28 additions & 14 deletions ...rg/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -588,20 +589,30 @@ public void deleteFromPolicyMappingRecordsInCurrentTxn(
@Override
public void deleteAllEntityPolicyMappingRecordsInCurrentTxn(
@Nonnull PolarisCallContext callCtx,
@Nonnull PolarisEntityCore entity,
@Nonnull PolarisBaseEntity entity,
@Nonnull List<PolarisPolicyMappingRecord> mappingOnTarget,
@Nonnull List<PolarisPolicyMappingRecord> 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} */
Expand Down Expand Up @@ -644,9 +655,12 @@ record -> this.store.getSlicePolicyMappingRecordsByPolicy().delete(record));
/** {@inheritDoc} */
@Override
public @Nonnull List<PolarisPolicyMappingRecord> loadAllTargetsOnPolicyInCurrentTxn(
@Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) {
@Nonnull PolarisCallContext callCtx,
long policyCatalogId,
long policyId,
int policyTypeCode) {
return this.store
.getSlicePolicyMappingRecordsByPolicy()
.readRange(this.store.buildPrefixKeyComposite(policyCatalogId, policyId));
.readRange(this.store.buildPrefixKeyComposite(policyTypeCode, policyCatalogId, policyId));
}
}
10 changes: 7 additions & 3 deletions polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -73,7 +73,7 @@ default void deleteFromPolicyMappingRecords(
*/
default void deleteAllEntityPolicyMappingRecords(
@Nonnull PolarisCallContext callCtx,
@Nonnull PolarisEntityCore entity,
@Nonnull PolarisBaseEntity entity,
@Nonnull List<PolarisPolicyMappingRecord> mappingOnTarget,
@Nonnull List<PolarisPolicyMappingRecord> mappingOnPolicy) {
throw new UnsupportedOperationException("Not Implemented");
Expand Down Expand Up @@ -141,11 +141,15 @@ default List<PolarisPolicyMappingRecord> 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<PolarisPolicyMappingRecord> loadAllTargetsOnPolicy(
@Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) {
@Nonnull PolarisCallContext callCtx,
long policyCatalogId,
long policyId,
int policyTypeCode) {
throw new UnsupportedOperationException("Not Implemented");
}
}
Loading
Loading