amogh-jahagirdar commented on code in PR #8860:
URL: https://github.com/apache/iceberg/pull/8860#discussion_r1362487232


##########
core/src/main/java/org/apache/iceberg/BaseFile.java:
##########
@@ -460,6 +460,12 @@ public ByteBuffer keyMetadata() {
 
   @Override
   public List<Long> splitOffsets() {
+    // If the last split offset is past the file size this means the split 
offsets are corrupted and
+    // should not be used
+    if (splitOffsets != null && splitOffsets[splitOffsets.length - 1] >= 
fileSizeInBytes) {

Review Comment:
   Actually, `TestManifestReader` seems better for verifying the split offsets 
are null when reading the file entries. This seems like a cleaner test since 
split planning tests already verify the behavior when explicit split offsets 
aren't defined. Manifest reader tests can help us explicitly verify the 
behavior when the offsets are corrupted.



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to