Re: [PR] Core: Handle empty tables in RESTCatalogAdapter for server side planning [iceberg]

["amogh-jahagirdar (via GitHub)" <gi...@apache.org>]

amogh-jahagirdar commented on code in PR #14660:
URL: https://github.com/apache/iceberg/pull/14660#discussion_r2553617054


##########
core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java:
##########
@@ -769,16 +769,22 @@ private static void planFilesFor(TableScan tableScan, String planId, int tasksPe
         Iterables.partition(tableScan.planFiles(), tasksPerPlanTask);
     int planTaskSequence = 0;
     String previousPlanTask = null;
+    String planTaskKeyPrefix = planId + "-" + tableScan.table().uuid() + "-";

Review Comment:
   I think I'd fix this a bit differently with a refactor. I think we can just make `planFilesFor` itself return the initialPlanTasks + next task Pair rather than just be void. Then I think we'd be able to get rid of this "populate and later find the initial based on the seq" logic. 
   
   In this refactored method, there'd be a short circuit logic which looks something like
   
   ```
   if (Iterables.isEmpty(taskGroupings)) {
      return Pair.of(Lists.empty(), null));
   }
   
   // The refactored logic so we keep track of the initial tasks and then later return them
   ```



##########
core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java:
##########
@@ -769,16 +769,22 @@ private static void planFilesFor(TableScan tableScan, String planId, int tasksPe
         Iterables.partition(tableScan.planFiles(), tasksPerPlanTask);
     int planTaskSequence = 0;
     String previousPlanTask = null;
+    String planTaskKeyPrefix = planId + "-" + tableScan.table().uuid() + "-";

Review Comment:
   let me know if that makes sense or not, something I was thinking through (and should've done to begin with in hindsight)



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

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


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