rdblue commented on a change in pull request #2865:
URL: https://github.com/apache/iceberg/pull/2865#discussion_r677026422
##########
File path: core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java
##########
@@ -62,6 +63,9 @@
ImmutableSet.of(DataOperations.OVERWRITE, DataOperations.REPLACE,
DataOperations.DELETE);
private static final Set<String>
VALIDATE_DATA_FILES_EXIST_SKIP_DELETE_OPERATIONS =
ImmutableSet.of(DataOperations.OVERWRITE, DataOperations.REPLACE);
+ // delete files are only added in "overwrite" operations
+ private static final Set<String> VALIDATE_REPLACED_DATA_FILES_OPERATIONS =
Review comment:
I'll update this. And I'll also add "delete" as an operation. Just
because we only create "overwrite" commits for row-level deletes today doesn't
mean that we won't create "delete" commits later. I considered updating the
`RowDelta` operation so that it created delete commits if only delete files
were added, but didn't end up doing it because we don't do that for overwrites
that only remove data files.
We can discuss whether to convert overwrite commits to delete commits that
later, but for now I think that we should add "delete" to this list because it
would be a valid commit. Just because the reference implementation doesn't
produce them doesn't mean it isn't valid.
--
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]
