jackye1995 commented on a change in pull request #4082:
URL: https://github.com/apache/iceberg/pull/4082#discussion_r803308200
##########
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:
> If you remove main then current-snapshot-id is -1 but we don't
actually have -1 in this map?
I remember we were thinking about deprecating `current-snapshot-id` after
branching is added to spec, because `current-snapshot-id` should now be
retrieved by getting the snapshot ID of the main branch, and that could be -1
if not exist. If we go with that route, `main` is always an entry in the map,
and we allow a `{"main": -1}` entry in the map and we can deprecate
`current-snapshot-id`. If we keep `current-snapshot-id`, then that information
of `{"main": -1}` is duplicate and we don't need to keep that. Either way is
fine to me, do you think any one of them has any particular advantage?
--
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]
