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, this sounds reasonable. 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 that it's a branch would happen
in the scan context.`useRef().asOfTime()` I don't think we would want the extra
method because time travel would only apply for branches so having the ref in
that case doesn't make sense to me since it's really only supported for 1 ref
type, the branch.
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]
