rdblue commented on code in PR #17541:
URL: https://github.com/apache/iceberg/pull/17541#discussion_r4087703394
##########
core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java:
##########
@@ -46,6 +46,15 @@ static DeleteFile asDVDeleteFile(TrackedFile file,
Map<Integer, PartitionSpec> s
return new TrackedDVDeleteFile(file, resolveSpecId(file, specsById));
}
+ static DeleteFile asDVDeleteFile(DataFile dataFile) {
+ Preconditions.checkArgument(
+ dataFile instanceof TrackedDataFile,
+ "Cannot create DV delete file from DataFile: %s",
Review Comment:
This error message contradicts the function definition, which creates a DV
`DeleteFile` from a `DataFile`. More information is needed, or this should use
a different adapter to always support the method contract, or this method
definition is not a good idea.
I think the right solution is the last one. We don't want to have an adapter
for `DataFile` with a DV to `DeleteFile` because we know that case will always
be a v4 co-located DV. We just also don't want a method definition that implies
any `DataFile` can be adapted. So the best solution is to move the conversion
into `FilePlanner`. That class knows when it is using a `DataFile` that was
adapted from `TrackedFile` and can unwrap it more safely. Then it can call
`asDVDeleteFile` without adding a new factory method.
--
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]
