singhpk234 commented on code in PR #14561:
URL: https://github.com/apache/iceberg/pull/14561#discussion_r2514944221
##########
core/src/main/java/org/apache/iceberg/rest/requests/PlanTableScanRequest.java:
##########
@@ -88,9 +88,17 @@ private PlanTableScanRequest(
@Override
public void validate() {
- Preconditions.checkArgument(
- snapshotId != null ^ (startSnapshotId != null && endSnapshotId !=
null),
- "Either snapshotId must be provided or both startSnapshotId and
endSnapshotId must be provided");
+ if (null != snapshotId) {
+ Preconditions.checkArgument(
+ null == startSnapshotId && null == endSnapshotId,
+ "Invalid scan: cannot provide both snapshotId and
startSnapshotId/endSnapshotId");
+ }
+
+ if (null != startSnapshotId || null != endSnapshotId) {
+ Preconditions.checkArgument(
+ null != startSnapshotId && null != endSnapshotId,
+ "Invalid incremental scan: startSnapshotId and endSnapshotId is
required");
+ }
Review Comment:
I agree with the change, since its well defined in the spec.
but i am unclear when would a client not be aware of the latest snapshot in
the main branch (assuming the scan is triggered from the table obj itself ?)
may be snapshot id is intended to be time travel use case, what i am thinking
is the planTable response we don't send back which snapshot id this responds
to, can it create confusion ?
##########
core/src/main/java/org/apache/iceberg/rest/requests/PlanTableScanRequest.java:
##########
@@ -117,7 +129,7 @@ public static class Builder {
private Long endSnapshotId;
private List<String> statsFields;
- public Builder() {}
+ private Builder() {}
Review Comment:
Lets mark this as deprecated then ? to be removed in next version and link
the builder() to it ?
--
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]
