singhpk234 commented on code in PR #14614:
URL: https://github.com/apache/iceberg/pull/14614#discussion_r2538453797


##########
core/src/main/java/org/apache/iceberg/rest/requests/PlanTableScanRequest.java:
##########
@@ -66,6 +67,10 @@ public List<String> statsFields() {
     return statsFields;
   }
 
+  public Integer minRowsRequested() {
+    return minRowsRequested;
+  }

Review Comment:
   minor : it would be nice to assert that the minRowsRequested when its not 
present in JSON the fromJSON created obj should return null.



##########
core/src/main/java/org/apache/iceberg/rest/requests/PlanTableScanRequest.java:
##########
@@ -74,7 +79,8 @@ private PlanTableScanRequest(
       boolean useSnapshotSchema,
       Long startSnapshotId,
       Long endSnapshotId,
-      List<String> statsFields) {
+      List<String> statsFields,
+      Integer minRowsRequested) {

Review Comment:
   Lets add a validation in `validate()` that it  should not be < 0, when 
non-null ? 



##########
core/src/main/java/org/apache/iceberg/rest/requests/PlanTableScanRequestParser.java:
##########
@@ -101,6 +106,7 @@ public static PlanTableScanRequest fromJson(JsonNode json) {
     Long snapshotId = JsonUtil.getLongOrNull(SNAPSHOT_ID, json);
     Long startSnapshotId = JsonUtil.getLongOrNull(START_SNAPSHOT_ID, json);
     Long endSnapshotId = JsonUtil.getLongOrNull(END_SNAPSHOT_ID, json);
+    Integer minRowsRequested = JsonUtil.getIntOrNull(MIN_ROWS_REQUESTED, json);

Review Comment:
   This LGTM, though one this is we are putting a MAX LIMIT value as INT_MAX 
now so if someone put LONG_MAX it will overflow and fail earlier ?
   which i think is consistent with what spark provides as PushDownLimit 
interface 
   
https://github.com/apache/spark/blob/master/sql/catalyst/src/main/java/org/apache/spark/sql/connector/read/SupportsPushDownLimit.java#L35



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