nastra commented on code in PR #9725:
URL: https://github.com/apache/iceberg/pull/9725#discussion_r1489631189
##########
core/src/main/java/org/apache/iceberg/view/ViewMetadata.java:
##########
@@ -479,21 +488,21 @@ public ViewMetadata build() {
metadataLocation);
}
- static List<ViewVersion> expireVersions(
- Map<Integer, ViewVersion> versionsById, int numVersionsToKeep) {
- // version ids are assigned sequentially. keep the latest versions by ID.
- List<Integer> ids = Lists.newArrayList(versionsById.keySet());
- ids.sort(Comparator.reverseOrder());
+ @VisibleForTesting
+ static List<ViewVersion> expireVersions(List<ViewVersion> versions, int
numVersionsToKeep) {
+ List<ViewVersion> versionsToKeep = Lists.newArrayList(versions);
- List<ViewVersion> retainedVersions = Lists.newArrayList();
- for (int idToKeep : ids.subList(0, numVersionsToKeep)) {
- retainedVersions.add(versionsById.get(idToKeep));
+ // version ids are assigned sequentially. keep the latest
numVersionsToKeep
+ if (numVersionsToKeep > 0 && versionsToKeep.size() > numVersionsToKeep) {
Review Comment:
not sure how defensive we want to be with this check but the method is
package-private so someone could actually use invalid values for
`numVersionsToKeep`
--
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]
