amogh-jahagirdar commented on code in PR #16957:
URL: https://github.com/apache/iceberg/pull/16957#discussion_r3825845760
##########
core/src/main/java/org/apache/iceberg/DeleteFileIndex.java:
##########
@@ -211,10 +226,34 @@ private DeleteFile findDV(long seq, DataFile dataFile) {
"DV data sequence number (%s) must be greater than or equal to data
file sequence number (%s)",
dv.dataSequenceNumber(),
seq);
+ validatePartitionMatch(dv, dataFile);
}
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(),
Review Comment:
I'm actually unsure about this check at read-time. The spec is really more a
positive statement on when deletes should apply, doesn't say that we need to
strictly fail in case of a spec mismatch; though I understand that this is
probably to protect against any potential correctness issues?
Basically, Is it worth failing the reads here? I can get behind validatin
within a commit on the write side because it would be unexpected to commit a
data file and a delete file for that data file which doesn't have that same
spec, but the spec basically does say simply in this case the delete file
doesn't apply to the 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]
