rambleraptor commented on code in PR #1477:
URL: https://github.com/apache/iceberg-go/pull/1477#discussion_r3605187846


##########
catalog/rest/scan_planning.go:
##########
@@ -187,9 +189,180 @@ func (r *Catalog) SupportsRemoteScanPlanning() bool {
 }
 
 // PlanFiles plans a scan server-side and returns tasks (and, optionally, a
-// plan-scoped FileIO) for the table to read.
+// plan-scoped FileIO) for the table to read. It submits the plan, polls a
+// submitted plan to completion, and expands any plan-task handles into their
+// tasks before returning.
+//
+// Decoding the returned tasks into table.FileScanTask is the one piece still
+// stubbed: RESTFileScanTask/RESTDeleteFile are empty pending the scan-task
+// decoder phase, so a plan that yields any task surfaces ErrNotImplemented 
(see
+// remoteScanTasks). SupportsRemoteScanPlanning therefore stays false until 
that
+// lands, so auto-mode scans keep planning locally rather than routing here.
 func (r *Catalog) PlanFiles(ctx context.Context, req 
table.ScanPlanningRequest) (table.ScanPlanningResult, error) {
-       return table.ScanPlanningResult{}, fmt.Errorf("%w: REST scan planning", 
iceberg.ErrNotImplemented)
+       wire, err := planTableScanRequestFrom(req)
+       if err != nil {
+               return table.ScanPlanningResult{}, err
+       }
+
+       resp, err := r.PlanTableScan(ctx, req.Identifier, wire)
+       if err != nil {
+               return table.ScanPlanningResult{}, err
+       }
+
+       // Resolve to a completed plan: use an inline result as-is, or poll a
+       // submitted one to completion. PlanTableScan maps failed to an error 
and
+       // rejects other statuses, so only completed/submitted reach here.
+       var completed CompletedPlanningResult
+       switch resp.Status {
+       case PlanStatusCompleted:
+               completed = CompletedPlanningResult{
+                       ScanTasks:          resp.ScanTasks,
+                       StorageCredentials: resp.StorageCredentials,
+               }
+       case PlanStatusSubmitted:
+               completed, err = r.WaitForPlan(ctx, req.Identifier, 
*resp.PlanID, WaitForPlanOptions{})
+               if err != nil {
+                       return table.ScanPlanningResult{}, err
+               }
+       default:
+               return table.ScanPlanningResult{}, fmt.Errorf(
+                       "%w: unexpected plan status %q from planTableScan", 
ErrRESTError, resp.Status)
+       }
+
+       files, deletes, err := r.collectScanTasks(ctx, req.Identifier, 
completed.ScanTasks)
+       if err != nil {
+               return table.ScanPlanningResult{}, err
+       }
+
+       tasks, err := remoteScanTasks(files, deletes)
+       if err != nil {
+               return table.ScanPlanningResult{}, err
+       }
+
+       return table.ScanPlanningResult{
+               Tasks: tasks,
+               IO:    planIOFromCredentials(completed.StorageCredentials, 
req.MetadataLocation),

Review Comment:
   I set a TODO to help determine this in a later phase if that works.



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