laskoviymishka commented on PR #3260: URL: https://github.com/apache/iceberg-rust/pull/3260#issuecomment-5845032312
@xanderbailey I think you're right on the direction. If this can only fail at `build()`, we probably have the wrong API shape. I'd lean toward separate entry points that resolve to a pinned snapshot and return the existing `TableScan`. That also lines up with `incremental_append_scan` in #2997. I don't think a selector on the builder really fixes it, since it can still be called twice and overwritten. So to me it's mostly between: - `scan_as_of_time` / `scan_as_of_snapshot_id` - one `scan(selection)` entry point I'd slightly prefer the separate constructors, but no strong opinion. I also wouldn't block this PR on it. My only ask is that if we're changing the scan API, we try to do it once. This PR and #2997 touch the same surface, so it'd be good to settle them together before the next release. -- 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]
