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

Reply via email to