amogh-jahagirdar commented on code in PR #16957:
URL: https://github.com/apache/iceberg/pull/16957#discussion_r3831725344
##########
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:
>we see it in existing DV's.
Yeah I can definitley get behind commit time validation, like we do when
merging potential duplicate DVs but I wasn't sure yet about read time. I'd say
+1 now. I think the change has good intent for inferring a bad table state. I
largely just wanted to reason about if there was any legitimate case where we'd
be improperly breaking reads, but no I don't think that's the case. Every kind
of compaction out there would be required to apply the deletes when producing
new files (even if the files have a new spec ID).
So I'm fairly confident that there's really no legitimate case where there
can be a position delete with a partition spec for a data file with a different
partition spec.
I think this is a case where having the implementation be stricter than
what's in the spec is a good idea. In case of a mismatch, it'll force readers
to repair their table before moving forward, which is a good thing because they
would've been ignoring deletes this time.
--
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]
