dimas-b commented on code in PR #1112:
URL: https://github.com/apache/polaris/pull/1112#discussion_r1983734386


##########
service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java:
##########
@@ -1650,15 +1664,42 @@ public List<GrantResource> 
listGrantsForCatalogRole(String catalogName, String c
    * @param entitiesMap map of entities
    * @param catalogId the id of the catalog of the entity we are looking for
    * @param id id of the entity we are looking for
+   * @param entityType
    * @return null if the entity does not exist
    */
   private @Nullable PolarisBaseEntity getOrLoadEntity(
-      @Nullable Map<Long, PolarisBaseEntity> entitiesMap, long catalogId, long 
id) {
+      @Nullable Map<Long, PolarisBaseEntity> entitiesMap,
+      long catalogId,
+      long id,
+      PolarisEntityType entityType) {
     return (entitiesMap == null)
-        ? metaStoreManager.loadEntity(getCurrentPolarisContext(), catalogId, 
id).getEntity()
+        ? metaStoreManager
+            .loadEntity(getCurrentPolarisContext(), catalogId, id, entityType)
+            .getEntity()
         : entitiesMap.get(id);
   }
 
+  private @Nullable PolarisBaseEntity getOrLoadEntityForGrant(
+      @Nullable Map<Long, PolarisBaseEntity> entitiesMap, PolarisGrantRecord 
record) {
+    if (entitiesMap != null) {
+      return entitiesMap.get(record.getSecurableId());
+    }
+
+    for (PolarisEntityType type : PolarisEntityType.values()) {

Review Comment:
   This is a valid point. However, I would not want to overload the 
`BasePersistence` interface give the on-going NoSQL implementation effort.
   
   I'm open to revisiting this after we have a working NoSQL backend. 



-- 
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