[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #5150: Spark Integration to read from Snapshot ref

[GitBox <gi...@apache.org>]

amogh-jahagirdar commented on code in PR #5150:
URL: https://github.com/apache/iceberg/pull/5150#discussion_r939704610


##########
spark/v3.1/spark/src/main/java/org/apache/iceberg/spark/source/SparkBatchQueryScan.java:
##########
@@ -61,22 +62,32 @@ class SparkBatchQueryScan extends SparkBatchScan {
 
     this.snapshotId = readConf.snapshotId();
     this.asOfTimestamp = readConf.asOfTimestamp();
+    this.snapshotRef = readConf.snapshotRef();
+
 
     if (snapshotId != null && asOfTimestamp != null) {
       throw new IllegalArgumentException(
           "Cannot scan using both snapshot-id and as-of-timestamp to select the table snapshot");
+    } else if (snapshotId != null && snapshotRef != null) {
+      throw new IllegalArgumentException(
+          "Cannot scan using both snapshot-id and snapshot-ref to select the table snapshot");
+    } else if(asOfTimestamp!= null && snapshotRef != null) {
+      throw new IllegalArgumentException(
+          "Cannot scan using both as-of-timestamp and snapshot-ref to select the table snapshot");

Review Comment:
   +1 time travel is supported for branches. 
   
   @rdblue @namrathamyske how do you feel about having a separate scan API useBranchAsOfTime https://github.com/apache/iceberg/pull/5364/files#diff-68f831bb319578a644bd1024f51d85a09284bb34ebc5d9b7608362b24caf441aR73 like in this PR? 
   
   Pro:
   
   Explicit API for time travel on a branch which seems more obvious to me than 
   
   `scan.useBranch("branch").asOfTime(someTimeMs)`
   
   Cons:
   
   It's another API and perhaps some users find the builder more obvious. validation could be handled in the scan context as Ryan pointed out.
   
   



-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org