[GitHub] [iceberg] jackye1995 commented on a diff in pull request #5364: Core, API: Support scanning from refs

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

jackye1995 commented on code in PR #5364:
URL: https://github.com/apache/iceberg/pull/5364#discussion_r979074115


##########
api/src/main/java/org/apache/iceberg/TableScan.java:
##########
@@ -39,13 +39,25 @@ public interface TableScan extends Scan<TableScan, FileScanTask, CombinedScanTas
    */
   TableScan useSnapshot(long snapshotId);
 
+  /**
+   * Create a new {@link TableScan} from this scan's configuration that will use the given
+   * reference.

Review Comment:
   nit: use the snapshot ID of the given reference?



##########
api/src/main/java/org/apache/iceberg/TableScan.java:
##########
@@ -39,13 +39,25 @@ public interface TableScan extends Scan<TableScan, FileScanTask, CombinedScanTas
    */
   TableScan useSnapshot(long snapshotId);
 
+  /**
+   * Create a new {@link TableScan} from this scan's configuration that will use the given
+   * reference.
+   *
+   * @param ref reference
+   * @return a new scan based on the given reference.
+   * @throws IllegalArgumentException if a reference with the given name could not be found
+   */
+  TableScan useRef(String ref);
+
   /**
    * Create a new {@link TableScan} from this scan's configuration that will use the most recent
-   * snapshot as of the given time in milliseconds.
+   * snapshot as of the given time in milliseconds on the branch in the scan or main if no branch is
+   * set.
    *
    * @param timestampMillis a timestamp in milliseconds.
    * @return a new scan based on this with the current snapshot at the given time
-   * @throws IllegalArgumentException if the snapshot cannot be found
+   * @throws IllegalArgumentException if the snapshot cannot be found or time travel is attempted on

Review Comment:
   I think the doc change here is no longer needed?



##########
core/src/main/java/org/apache/iceberg/BaseTableScan.java:
##########
@@ -85,7 +85,7 @@ public TableScan appendsAfter(long fromSnapshotId) {
   @Override
   public TableScan useSnapshot(long scanSnapshotId) {
     Preconditions.checkArgument(
-        snapshotId() == null, "Cannot override snapshot, already set to id=%s", snapshotId());
+        snapshotId() == null, "Cannot override snapshot, already set snapshot id=%s", snapshotId());

Review Comment:
   nit: "set to snapshot id=%s"?



-- 
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