pvary commented on code in PR #14264:
URL: https://github.com/apache/iceberg/pull/14264#discussion_r2446784742
##########
core/src/main/java/org/apache/iceberg/BaseIncrementalChangelogScan.java:
##########
@@ -71,6 +82,14 @@ protected CloseableIterable<ChangelogScanTask> doPlanFiles(
.filter(manifest ->
changelogSnapshotIds.contains(manifest.snapshotId()))
.toSet();
+ // Build per-snapshot delete file indexes for added deletes
+ Map<Long, DeleteFileIndex> addedDeletesBySnapshot =
buildAddedDeleteIndexes(changelogSnapshots);
+
+ // Build delete file index for existing deletes (before the start
snapshot) if we have
+ // EQUALITY_DELETES files in the changelog range
+ DeleteFileIndex existingDeleteIndex =
+ buildExistingDeleteIndex(fromSnapshotIdExclusive,
addedDeletesBySnapshot);
Review Comment:
My main point was that we might need the existing deletes even when no
equality delete is present.
> We shouldn't have overlapping positional deletes
@danielcweeks: Is this specifically forbidden by the spec, or it is just not
reasonable, and it is probably not done by anyone? The only place where I can
imagine this happening when we have concurrent deletes, and we decided that
they can be committed without change as both of them are only deletes.
--
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]
