amogh-jahagirdar commented on code in PR #12670:
URL: https://github.com/apache/iceberg/pull/12670#discussion_r2023796463
##########
core/src/main/java/org/apache/iceberg/MetadataUpdateParser.java:
##########
@@ -150,7 +148,7 @@ private MetadataUpdateParser() {}
.put(MetadataUpdate.SetPartitionStatistics.class,
SET_PARTITION_STATISTICS)
.put(MetadataUpdate.RemovePartitionStatistics.class,
REMOVE_PARTITION_STATISTICS)
.put(MetadataUpdate.AddSnapshot.class, ADD_SNAPSHOT)
- .put(MetadataUpdate.RemoveSnapshot.class, REMOVE_SNAPSHOTS)
+ .put(MetadataUpdate.RemoveSnapshots.class, REMOVE_SNAPSHOTS)
Review Comment:
I'm not sure if this is what was meant by the first alternative presented
but I think we could just have 2 cases in the interim (until RemoveSnapshot is
deprecated)
```
.put(MetadataUpdate.RemoveSnapshots.class, REMOVE_SNAPSHOTS)
.put(MetadataUpdate.RemoveSnapshot.class, REMOVE_SNAPSHOTS)
```
When serializing the update, we'd be able to distinguish if it's
removeSnapshot or RemoveSnapshots. you could update writeRemoveSnapshots to
take in just a RemoveSnapshots instance, and for the RemoveSnapshot case, you'd
construct a RemoveSnapshots passing in a list with a single snapshot element.
Then in the future once RemoveSnapshot is removed after deprecation, we
could then just remove the additional handling for that case, and update the
tests which have some expectations on the produced update types.
Let me know if that makes sense or not!
--
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...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
