eric-maynard commented on code in PR #1686:
URL: https://github.com/apache/polaris/pull/1686#discussion_r2122664135


##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java:
##########
@@ -403,6 +404,20 @@ boolean hasChildren(
       long catalogId,
       long parentId);
 
+  /**
+   * Check if the specified IcebergTableLikeEntity has any same-namespace 
siblings which share a
+   * location
+   *
+   * @param callContext the polaris call context
+   * @param parentId the parent entity to look for duplicates inside
+   * @param location the location to check for overlaps against
+   * @return Optional.of(Optional.of(location)) if the parent entity has 
children,
+   *     Optional.of(Optional.empty()) if not, and Optional.empty() if the 
metastore doesn't support
+   *     this operation
+   */
+  Optional<Optional<String>> hasOverlappingSiblings(
+      @Nonnull PolarisCallContext callContext, long parentId, String location);

Review Comment:
   > This method implies that all Persistence implementations has a common 
understanding of what location means in entities.
   
   I don't think that's exactly true. There's nothing here that implies 
commonality across persistence implementations. One persistence implementation 
could have `location` as the base location, while another persistence 
implementation could have the base location flipped backwards. Both 
implementations could work.
   
   >  Ideally, since it is used as a first interface parameter, it should be 
exposes as a first class accessor method in entities.
   
   The entities that have a location do expose a method to get the location -- 
`getBaseLocation`. There's no interface for this method. I would love to see 
this refactored, and then this method could take a `EntityWithLocation` or 
whatever interface we choose to have the location-having entities implement. 
But since we don't use those types consistently now, I think that change would 
be quite difficult to make.



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