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

Reply via email to