kbendick commented on code in PR #4632:
URL: https://github.com/apache/iceberg/pull/4632#discussion_r859345921


##########
core/src/main/java/org/apache/iceberg/MetadataUpdateParser.java:
##########
@@ -140,21 +170,73 @@ public static MetadataUpdate fromJson(JsonNode jsonNode) {
         throw new UnsupportedOperationException("Not implemented: AssignUUID");
       case UPGRADE_FORMAT_VERSION:
         return readAsUpgradeFormatVersion(jsonNode);
+      case ADD_SCHEMA:
+        return readAsAddSchema(jsonNode);
+      case SET_CURRENT_SCHEMA:
+        return readAsSetCurrentSchema(jsonNode);
+      case SET_DEFAULT_PARTITION_SPEC:
+        return readAsSetDefaultPartitionSpec(jsonNode);
+      case ADD_PARTITION_SPEC:
+      case ADD_SORT_ORDER:
+      case SET_DEFAULT_SORT_ORDER:
+      case ADD_SNAPSHOT:
+      case REMOVE_SNAPSHOTS:

Review Comment:
   So I had the same thought, but in the spec, the action is defined as 
`remove-snapshots`.
   
   If you look at the implementation, it immediately converts the single 
snapshot ID to a set (which is what the spec defines).
   
   So it might make more sense to eventually rename the class `RemoveSnapshots` 
and/or add a constructor that takes in a collection / iterable. That's what I 
intend to do in one of my follow up PRs that implements the serde for this 
class.
   
   Good catch though!



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