rahil-c commented on code in PR #9695:
URL: https://github.com/apache/iceberg/pull/9695#discussion_r1517180435


##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2838,6 +3093,59 @@ components:
           additionalProperties:
             type: string
 
+    PreplanTableRequest:
+      type: object
+      required:
+        - select
+        - filter
+        - case-sensitive
+      properties:
+        select:
+          description: A list of the selected column names
+          type: array
+          items:
+            type: string
+        filter:
+          $ref: '#/components/schemas/Expression'
+        case-sensitive:
+          description: Indicates whether column selection and filtering should 
be case sensitive
+          type: boolean
+        snapshot-id:
+          description: an int64 snapshot ID (if snapshot-range is not 
present); optional and defaults to the table's current snapshot
+          type: integer
+          format: int64
+        snapshot-range:
+          description: a JSON list containing exactly 2 int64 snapshot IDs (if 
snapshot-id is not present) representing the start (exclusive) and end 
(inclusive) snapshots

Review Comment:
   > Isn't a range typically inclusive start and exclusive end? why it is 
reversed here? I am starting to feel maybe using start-snapshot-id and 
end-snapshot-id might be a better choice to be less ambiguous, and those values 
can also be null to indicate no start or no end.
   
   I think @stevenzwu had also brought this up for the `IncrementalScan` for 
having inclusive and exclusive for the from/start snapshot. 
https://github.com/apache/iceberg/blob/main/api/src/main/java/org/apache/iceberg/IncrementalScan.java#L34
   
   On further looking at the incrementalScan it seems that for to/end snapshot 
it is only inclusive 
https://github.com/apache/iceberg/blob/main/api/src/main/java/org/apache/iceberg/IncrementalScan.java#L87
   
   ```
     ThisT toSnapshot(long toSnapshotId);
   
     /**
      * Instructs this scan to look for changes up to a particular snapshot ref 
(inclusive
      * 
   ```
   
   So should we have a boolean flag to tell if from/start is inclusive or 
exclusive? And for end we always make it inclusive?
    



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

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


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

Reply via email to