amogh-jahagirdar commented on code in PR #15006:
URL: https://github.com/apache/iceberg/pull/15006#discussion_r2848002475
##########
core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java:
##########
@@ -86,8 +91,8 @@ abstract class MergingSnapshotProducer<ThisT> extends
SnapshotProducer<ThisT> {
// update data
private final Map<Integer, DataFileSet> newDataFilesBySpec =
Maps.newHashMap();
private Long newDataFilesDataSequenceNumber;
- private final Map<Integer, DeleteFileSet> newDeleteFilesBySpec =
Maps.newHashMap();
- private final Set<String> newDVRefs = Sets.newHashSet();
+ private final List<DeleteFile> v2Deletes = Lists.newArrayList();
+ private final Map<String, List<DeleteFile>> dvsByReferencedFile =
Maps.newLinkedHashMap();
Review Comment:
I commented this elsehwere but the reason this field is a `LinkedHashMap` is
because there's quite a lot of tests which verify the exact ordering of entries
in a manifest. I do think we should change those tests because we really
shouldn't be guaranteeing any ordering in the entries in the output manifests
but that's a larger change. The current implementation which tracks
`newDeleteFilesBySpec` uses a DeleteFileSet which is an insertion-ordering
preserving structure.
--
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]
