wypoon commented on a change in pull request #1508:
URL: https://github.com/apache/iceberg/pull/1508#discussion_r497140645
##########
File path: api/src/main/java/org/apache/iceberg/Table.java
##########
@@ -51,6 +51,23 @@
*/
Schema schema();
+ /**
+ * Return the {@link Schema schema} for this table at the time of the
snapshot
+ * specified by the snapshotId.
+ *
+ * @return this table's schema
+ */
+ Schema schemaForSnapshot(long snapshotId);
+
+ /**
+ * Return the {@link Schema schema} for this table at the time of the most
recent
+ * snapshot as of the specified timestampMillis.
+ * Note: This is not necessarily the schema at the time of the specified
timestampMillis.
+ *
+ * @return this table's schema
+ */
+ Schema schemaForSnapshotAsOfTime(long timestampMillis);
Review comment:
I can avoid adding the `schemaForSnaphot` methods to the `Table` API. I
think only `BaseTable` needs to have the methods. However, the places where I
need to call `schemaForSnapshot` have references to simply `Table`; I'd have to
check if the `Table` is an instance of `BaseTable`.
I also suggested in the general comments that `Snapshot` could simply have a
`schemaId` method in the future. And that would then get used in a revised
implementation of `schemaForSnapshot` in `BaseTable`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
