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