aokolnychyi 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_r331814993
########## 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: This one is related to other comments: I think it makes sense for utilities to read only live entries from manifests and make sure they change the status of entries to `EXISTING`. This method can stay general-purpose in case we need it later (we are already using it for appends and rewrites). ---------------------------------------------------------------- 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