amogh-jahagirdar commented on code in PR #8860:
URL: https://github.com/apache/iceberg/pull/8860#discussion_r1362567101
##########
core/src/main/java/org/apache/iceberg/BaseFile.java:
##########
@@ -460,6 +460,16 @@ public ByteBuffer keyMetadata() {
@Override
public List<Long> splitOffsets() {
+ if (splitOffsets == null || splitOffsets.length == 0) {
+ return null;
+ }
+
+ // If the last split offset is past the file size this means the split
offsets are corrupted and
Review Comment:
Do you mean throwing at read time (i.e. instead of returning null, we
throw)? If so, I don't think we want to do that because that's essentially
unnecessarily breaking future readers and split offsets are optional anyways;
this approach takes the stance of detecting the corruption and making sure read
logic doesn't leverage the corrupted metadata.
If you mean throwing at the time of writing the file (a precondition check
in the constructor of `BaseFile`), I went back and forth on this but I think
the problem there is let's say someone upgrades. When some process is performed
which rewrites a set of files (including some corrupted entries) it would fail
due to the precondition. The benefit is it would prevent spreading the previous
corruption which is nice, but at the cost of failing operations. Considering
again the corrupted split offsets will be ignored at read time anyways, failing
at write time seems needless.
To prevent spreading previous corrupted state, at the time of writing the
manifest if the corruption is detected the split offsets could be recomputed (a
sort of "fix-up" process). This requires more investigation though, not sure
how feasible it is and the perf implications (e.g. for Parquet we'd need to go
through the block metadata again)
--
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
