HonahX commented on code in PR #1314: URL: https://github.com/apache/polaris/pull/1314#discussion_r2029560502
########## 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: > How about letting it extends PolarisException? I'm trying to avoid adding a new exception. I'm fine with reuse this one. I was also thinking about move this exception into the package policy.exceptions, but defer to a followup for that. Sounds good, let's first extend the PolarisException and move in a later 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