amogh-jahagirdar commented on code in PR #10721: URL: https://github.com/apache/iceberg/pull/10721#discussion_r1710093143
########## aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java: ########## @@ -420,4 +427,40 @@ protected void finalize() throws Throwable { } } } + + @Override + public boolean recoverFile(String path) { + S3URI location = new S3URI(path, s3FileIOProperties.bucketToAccessPointMapping()); + ListObjectVersionsIterable response = + client() + .listObjectVersionsPaginator( + ListObjectVersionsRequest.builder() + .bucket(location.bucket()) + .prefix(location.key()) + .build()); + + Optional<ObjectVersion> latestVersion = + response.versions().stream().max(Comparator.comparing(ObjectVersion::lastModified)); + return latestVersion.map(version -> recoverObject(version, location.bucket())).orElse(false); Review Comment: Yeah that seems like a simple check we can do to avoid unnecessary copies, updated! I didn't go down the route of comparing the latest deletion marker with the version to recover to, because inverting the logic to just verify that the version to recover to is not the latest is simple; and we know that "version to recover" cannot represent a deletion marker due to the guarantees provided by the S3 api. -- 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