huaxingao commented on code in PR #15727:
URL: https://github.com/apache/iceberg/pull/15727#discussion_r3859453331
##########
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:
It does change #16957's behavior. that PR is titled "Fail scans when
position deletes/DVs don't match data file partition," and this inverts all
four of its tests. Could we ask its author and reviewers to weigh in?
This may also not be needed. `validatePartitionMatch` is only reached from
`findPathDeletes` and `findDV`, since the partition keyed lookups already match
on the data file's own partition. Equality deletes never reach it, so your 6.3b
to 3.2b result should hold without this change.
--
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]
