rdblue commented on code in PR #16867:
URL: https://github.com/apache/iceberg/pull/16867#discussion_r3502439137
##########
core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java:
##########
@@ -405,6 +417,121 @@ public DeleteFile copyWithStats(Set<Integer>
requestedColumnIds) {
}
}
+ /**
+ * Adapts a TrackedFile DATA_MANIFEST or DELETE_MANIFEST entry to the {@link
ManifestFile}
+ * interface.
+ */
+ private static class TrackedManifestFile implements ManifestFile {
+ private final TrackedFile file;
+
+ private TrackedManifestFile(TrackedFile file) {
+ Preconditions.checkArgument(
+ file.tracking().dataSequenceNumber() != null, "Invalid data sequence
number: null");
+ Preconditions.checkArgument(
+ file.tracking().snapshotId() != null, "Invalid snapshot ID: null");
+ Preconditions.checkArgument(
+ file.manifestInfo().dv() == null,
+ "Cannot adapt manifest with a deletion vector to ManifestFile: %s",
Review Comment:
This is correct and calls into question whether this adapter is going to be
useful. We expect manifest DVs to be common and if we can't adapt a manifest
with a DV back to `ManifestFile`, then what is the purpose of this adapter?
`ManifestFile` is mostly below the API level. It is exposed in some places
(accessed primarily from `Snapshot`), but reading and writing for correctness
(scan planning and commit operations) are all done within `Table` APIs and even
snapshot change access is moving to `SnapshotChanges`. You can read manifests
directly using utils in core, but I don't think that is very common.
I think we may want to avoid adding this for now, even though it seems like
a reasonable addition. In order for this to be valuable, we would need to
continue using `ManifestFile` and expose a DV through that interface. Since we
don't know if that's going to be helpful, I think we should keep this as a
draft for now.
--
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]
