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

Reply via email to