kinolaev commented on code in PR #15727:
URL: https://github.com/apache/iceberg/pull/15727#discussion_r3851337932
##########
core/src/main/java/org/apache/iceberg/DeleteFileIndex.java:
##########
@@ -220,38 +222,26 @@ private DeleteFile findDV(long seq, DataFile dataFile) {
}
DeleteFile dv = dvByPath.get(dataFile.location());
- if (dv != null) {
- ValidationException.check(
- dv.dataSequenceNumber() >= seq,
- "DV data sequence number (%s) must be greater than or equal to data
file sequence number (%s)",
- dv.dataSequenceNumber(),
- seq);
- validatePartitionMatch(dv, dataFile);
+ if (dv != null && (dv.dataSequenceNumber() < seq ||
!validatePartitionMatch(dv, dataFile))) {
+ return null;
}
return dv;
}
- private void validatePartitionMatch(DeleteFile deleteFile, DataFile
dataFile) {
- ValidationException.check(
- deleteFile.specId() == dataFile.specId(),
- "Mismatched partition specs (%s, %s) for delete file %s and data file
%s:"
- + " metadata is corrupted",
- deleteFile.specId(),
- dataFile.specId(),
- deleteFile.location(),
- dataFile.location());
+ private boolean validatePartitionMatch(DeleteFile deleteFile, DataFile
dataFile) {
+ if (deleteFile.specId() != dataFile.specId()) {
+ // Mismatched partition specs for delete file and data file: metadata is
corrupted
+ return false;
+ }
if (partitionComparatorsBySpecId != null) {
Comparator<StructLike> partitionComparator =
partitionComparatorsBySpecId.get(deleteFile.specId());
- ValidationException.check(
- partitionComparator.compare(deleteFile.partition(),
dataFile.partition()) == 0,
- "Mismatched partition tuples (%s, %s) for delete file %s and data
file %s:"
- + " metadata is corrupted",
- deleteFile.partition(),
- dataFile.partition(),
- deleteFile.location(),
- dataFile.location());
+ if (partitionComparator.compare(deleteFile.partition(),
dataFile.partition()) != 0) {
+ // Mismatched partition tuples for delete file and data file: metadata
is corrupted
+ return false;
Review Comment:
No, it doesn't, it just skips a delete file in case of partition mismatch.
Check out the updated test - instead of throwing an exception, `task.deletes()`
is empty. This matches the relaxation we discussed back in April.
The spec says nothing about failure, it says:
> A position delete file must be applied to a data file when all of the
following are true:
> The data file's file_path is equal to the delete file's
referenced_data_file if it is non-null
> The data file's partition (both spec and partition values) is equal to the
delete file's partition
So, there is no spec violation here: a position delete file with a different
partition won't be applied to a data file.
--
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]
