dhruvarya-db commented on PR #3260:
URL: https://github.com/apache/iceberg-rust/pull/3260#issuecomment-5823999993

   @xanderbailey Do any of these make sense:
   
   1. Fail in `snapshot_id()` or `as_of_time()` itself if the other has already 
been called instead of failing during `build()`
   2. Let the second invocation simply override the first one (i.e. snapshot_id 
will clear the timestamp while as_of_time will clear the explicitly passed id. 
This one seems a bit risky.
   3. The enum approach from your suggestion:
   ```
   enum SnapshotSelection {
       Current,
       SnapshotId(i64),
       AsOfTime(i64),
   }
   table.scan()
       .with_snapshot_selection(
           SnapshotSelection::AsOfTime(timestamp_ms)
       )
       .build()?;
   ```
   A problem with this is that the user can first invoke 
with_snapshot_selection(SnapshotSelection::AsOfTime(timestamp_ms)) followed by 
with_snapshot_selection(SnapshotSelection::SnapshotId(id)) within the same 
build chain. So it doesn't really solve our issue.
   4. The new scan type solves all of these issues but I am not sure whether it 
is the most ergonomic solution:
   ```
   table.scan_as_of_time(timestamp_ms)
       .select(["x"])
       .build()?;
    table.scan_as_of_snapshot_id(snapshot_id)
       .select(["x"])
       .build()?;
   ```
   
   
   Option 1 seems like the simplest one here but I have no strong opinion here. 
Option 4 seems to solve the main issues.


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

Reply via email to