aokolnychyi commented on a change in pull request #3069:
URL: https://github.com/apache/iceberg/pull/3069#discussion_r712522599
##########
File path: core/src/main/java/org/apache/iceberg/BaseOverwriteFiles.java
##########
@@ -127,8 +131,17 @@ protected void validate(TableMetadata base) {
}
}
+ // validating concurrently added data files is optional and is only needed
to achieve serializable isolation
if (conflictDetectionFilter != null && base.currentSnapshot() != null) {
validateAddedDataFiles(base, startingSnapshotId,
conflictDetectionFilter, caseSensitive);
}
+
+ // validating concurrently added delete files is required whenever we
overwrite specific data files
+ // if we find a new delete file matching one of the data files we are
trying to overwrite,
+ // we must fail this operation as it would undelete rows that were removed
concurrently
+ if (deletedDataFiles.size() > 0) {
+ validateNoNewDeletesForDataFiles(
+ base, startingSnapshotId, conflictDetectionFilter, deletedDataFiles,
caseSensitive);
Review comment:
Okay, if it is something we want to support, it means there are 3 cases:
- Case 1: copy-on-write MERGE -> we must validate deletes.
- Case 2: overwrite by filter with serializable isolation -> we must
validate deletes.
- Case 3: overwrite by filter with snapshot isolation -> we must NOT
validate deletes.
Since delete validation becomes optional, it probably means we need a
separate method to trigger it.
Thoughts, @rdblue @szehon-ho?
--
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]
