rdblue commented on a change in pull request #512: Extend RewriteManifests with 
a way to add/delete manifests
URL: https://github.com/apache/incubator-iceberg/pull/512#discussion_r332292119
 
 

 ##########
 File path: core/src/main/java/org/apache/iceberg/ManifestWriter.java
 ##########
 @@ -36,14 +38,33 @@
 
   static ManifestFile copyAppendManifest(ManifestReader reader, OutputFile 
outputFile, long snapshotId,
                                          SnapshotSummary.Builder 
summaryBuilder) {
+    return copyManifest(reader, outputFile, snapshotId, summaryBuilder, 
Sets.newHashSet(ManifestEntry.Status.ADDED));
+  }
+
+  static ManifestFile copyManifest(ManifestReader reader, OutputFile 
outputFile, long snapshotId,
+                                   SnapshotSummary.Builder summaryBuilder,
+                                   Set<ManifestEntry.Status> 
allowedEntryStatuses) {
     ManifestWriter writer = new ManifestWriter(reader.spec(), outputFile, 
snapshotId);
     boolean threw = true;
     try {
       for (ManifestEntry entry : reader.entries()) {
-        Preconditions.checkArgument(entry.status() == 
ManifestEntry.Status.ADDED,
-            "Cannot append manifest: contains existing files");
-        summaryBuilder.addedFile(reader.spec(), entry.file());
-        writer.add(entry);
+        Preconditions.checkArgument(
+            allowedEntryStatuses.contains(entry.status()),
+            "Invalid manifest entry status: %s (allowed statuses: %s)",
+            entry.status(), allowedEntryStatuses);
+        switch (entry.status()) {
+          case ADDED:
+            summaryBuilder.addedFile(reader.spec(), entry.file());
+            writer.add(entry);
+            break;
+          case EXISTING:
+            writer.existing(entry);
+            break;
+          case DELETED:
+            summaryBuilder.deletedFile(reader.spec(), entry.file());
 
 Review comment:
   `ManifestReader.entries` returns all entries unmodified. We could use live 
entries to discard the deletes. That's a good idea, but we would still need to 
handle converting adds to existing.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to