flyrain commented on code in PR #1314: URL: https://github.com/apache/polaris/pull/1314#discussion_r2029528647
########## service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java: ########## @@ -251,25 +248,224 @@ public Policy updatePolicy( public boolean dropPolicy(PolicyIdentifier policyIdentifier, boolean detachAll) { // TODO: Implement detachAll when we support attach/detach policy - PolarisResolvedPathWrapper resolvedEntities = + var resolvedPolicyPath = getResolvedPathWrapper(policyIdentifier); + var catalogPath = resolvedPolicyPath.getRawParentPath(); + var policyEntity = resolvedPolicyPath.getRawLeafEntity(); + + return metaStoreManager + .dropEntityIfExists( + callContext.getPolarisCallContext(), + PolarisEntity.toCoreList(catalogPath), + policyEntity, + Map.of(), + false) + .isSuccess(); + } + + public boolean attachPolicy( + PolicyIdentifier policyIdentifier, + PolicyAttachmentTarget target, + Map<String, String> parameters) { + + var resolvedPolicyPath = getResolvedPathWrapper(policyIdentifier); + var policyCatalogPath = PolarisEntity.toCoreList(resolvedPolicyPath.getRawParentPath()); + var policyEntity = PolicyEntity.of(resolvedPolicyPath.getRawLeafEntity()); + + var resolvedTargetPath = getResolvedPathWrapper(target); + var targetCatalogPath = PolarisEntity.toCoreList(resolvedTargetPath.getRawParentPath()); + var targetEntity = resolvedTargetPath.getRawLeafEntity(); + + PolicyValidators.validateAttach(policyEntity, targetEntity); + + var result = + metaStoreManager.attachPolicyToEntity( + callContext.getPolarisCallContext(), + targetCatalogPath, + targetEntity, + policyCatalogPath, + policyEntity, + parameters); + + if (result.getReturnStatus() == POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS) { + var targetId = catalogEntity.getName(); + if (target.getType() != CATALOG) { + targetId += "." + String.join(".", target.getPath()); + } + throw new PolicyMappingAlreadyExistsException( Review Comment: A side note: I also think it's OK to populate persistence layer exception here. Everything is wrapped in a result now. I feel like this isn't necessary. It's a big change though, which is out of scope of this PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@polaris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org