amogh-jahagirdar commented on a change in pull request #4082:
URL: https://github.com/apache/iceberg/pull/4082#discussion_r803290070



##########
File path: core/src/main/java/org/apache/iceberg/TableMetadata.java
##########
@@ -725,6 +728,21 @@ private static SortOrder freshSortOrder(int orderId, 
Schema schema, SortOrder so
     return builder.build();
   }
 
+  private Map<String, SnapshotRef> validateRefs(Map<String, SnapshotRef> 
inputRefs) {
+    for (Map.Entry<String, SnapshotRef> refEntry : inputRefs.entrySet()) {
+      String refName = refEntry.getKey();
+      SnapshotRef ref = refEntry.getValue();
+      if (ref.snapshotId() == -1) {
+        Preconditions.checkArgument(refName.equals(SnapshotRef.MAIN_BRANCH),
+            "Snapshot ref %s refers to snapshot id -1. Only main can refer to 
-1", refName);

Review comment:
       Removed the need for a special error message.
   
   Will let @jackye1995 comment, but would we be putting any implications (i.e. 
potentially unwanted behaviors) on retention if we allowed main to be dropped? 
If we allow main to be dropped this means that we must have a fallback 
mechanism for any other reference types (which would be the retention already 
defined at the table level). 
   
   This seems like it should be okay, but just trying to think through if there 
are any cases where allowing someone to drop main leads to any undesired 
behavior in retention.




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to