rdblue commented on code in PR #8673:
URL: https://github.com/apache/iceberg/pull/8673#discussion_r1348100841
##########
core/src/main/java/org/apache/iceberg/GenericManifestFile.java:
##########
@@ -84,15 +84,24 @@ public GenericManifestFile(Schema avroSchema) {
}
}
+ /**
+ * @deprecated since 1.5.0, will be removed in 1.6.0. Use {@link
+ * GenericManifestFile#GenericManifestFile(InputFile, int, Long)} with a
snapshot-id
+ */
+ @Deprecated
GenericManifestFile(InputFile file, int specId) {
+ this(file, specId, null);
+ }
+
+ GenericManifestFile(InputFile file, int specId, Long snapshotId) {
Review Comment:
I think I understand the rationale for adding this, but I'm pretty sure that
we didn't do it this way on purpose. By adding this option, you can easily pass
in the snapshot ID for a manfiest file so that you're deciding when it is
created what snapshot it has.
This is something that we wanted to avoid so that people didn't accidentally
corrupt metadata by setting the snapshot ID directly. Instead, callers can only
construct manifests with snapshot IDs that will be inherited from manifest list
metadata. At least, that's the idea. It's probably possible to work around it,
but I think it is more correct for people to not have the option when creating
the manifest metadata.
Also, I think this is something we use so that you can write a manifest of
files (like how Flink stores checkpoint state) and later add it to the table
once the snapshot ID is known.
--
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
