amogh-jahagirdar commented on code in PR #5150:
URL: https://github.com/apache/iceberg/pull/5150#discussion_r940852054
##########
api/src/main/java/org/apache/iceberg/TableScan.java:
##########
@@ -49,6 +49,16 @@ public interface TableScan extends Scan<TableScan,
FileScanTask, CombinedScanTas
*/
TableScan asOfTime(long timestampMillis);
+ /**
+ * Create a new {@link TableScan} from this scan's configuration that will
use the most recent
+ * snapshot as of the given snapshot ref.
+ *
+ * @param snapshotRef a snapshot Ref
+ * @return a new scan based on this with the given snapshot Ref
+ * @throws IllegalArgumentException if the snapshot cannot be found
+ */
+ TableScan useSnapshotRef(String snapshotRef);
Review Comment:
Sure. The only thing is I think if we do useRef (or if we come up with a
better name) then we would not want to have the `useRef(String refName, Long
timeStampMillis)`. A user would chain it with the existing useTimestamp and
then the validation would happen in the scan context. I don't think we would
want the extra method because time travel would only apply for branches so
having useRef in that case doesn't make sense to me.
If we have consensus on this, then I can update
https://github.com/apache/iceberg/pull/5364/files with the updated approach.
Then this, PR could be focused on the Spark side of the integration. Will wait
to hear what @aokolnychyi suggests as well!
--
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]
