Revanth14 commented on code in PR #1324:
URL: https://github.com/apache/iceberg-go/pull/1324#discussion_r3573146327
##########
catalog/rest/scan_planning.go:
##########
@@ -79,30 +125,125 @@ func (r *Catalog) PlanFiles(ctx context.Context, req
table.ScanPlanningRequest)
// --- Low-level client methods -----------------------------------------------
-// PlanTableScan submits a scan plan. The result is either completed inline,
-// submitted (returns a plan-id to poll), or failed.
+// PlanTableScan submits a scan plan. A completed or submitted plan returns
+// (resp, nil); callers branch on resp.Status for the plan-id (completed) vs
+// poll (submitted) distinction. A failed plan returns a zero response and a
+// non-nil *PlanFailedError (errors.Is(err, ErrPlanFailed)) so the if-err idiom
+// does not mistake a failure for success; the server detail rides on the
error.
func (r *Catalog) PlanTableScan(ctx context.Context, ident table.Identifier,
req PlanTableScanRequest) (PlanTableScanResponse, error) {
- return PlanTableScanResponse{}, fmt.Errorf("%w: plan table scan",
iceberg.ErrNotImplemented)
+ if err := r.endpoints.check(endpointPlanTableScan); err != nil {
+ return PlanTableScanResponse{}, err
+ }
+
+ path, err := r.scanPlanningPath(endpointPlanTableScan, ident)
+ if err != nil {
+ return PlanTableScanResponse{}, err
+ }
+
+ headers, err := scanPlanningHeaders(req.IdempotencyKey,
req.AccessDelegation, true)
+ if err != nil {
+ return PlanTableScanResponse{}, err
+ }
+
+ resp, err := doPost[PlanTableScanRequest, PlanTableScanResponse](
+ ctx, r.baseURI, path, req, r.cl,
+ map[int]error{http.StatusNotFound: catalog.ErrNoSuchTable},
withHeaders(headers))
+ if err != nil {
+ return PlanTableScanResponse{}, err
+ }
+ if resp.Status == PlanStatusFailed {
+ return PlanTableScanResponse{}, &PlanFailedError{Detail:
resp.Error}
+ }
+
+ return resp, nil
}
// FetchPlanningResult polls a previously submitted plan. opts.AccessDelegation
// is sent as the X-Iceberg-Access-Delegation header so an async poll can still
// receive plan-scoped storage credentials: the spec defines data-access on
this
// endpoint, and the completed-async result is where those credentials are
vended.
+//
+// completed and submitted return (resp, nil) and callers branch on resp.Status
+// (done vs poll again). The terminal failure states surface as errors so an
+// if-err poll loop cannot mistake them for an empty scan: failed returns a
+// *PlanFailedError (errors.Is(err, ErrPlanFailed)) and cancelled returns
+// ErrPlanCancelled. A 404 maps to ErrPlanExpired (the plan-id the server
forgot).
func (r *Catalog) FetchPlanningResult(ctx context.Context, ident
table.Identifier, planID string, opts FetchPlanningResultOptions)
(FetchPlanningResultResponse, error) {
- return FetchPlanningResultResponse{}, fmt.Errorf("%w: fetch planning
result", iceberg.ErrNotImplemented)
+ if err := r.endpoints.check(endpointFetchPlanResult); err != nil {
+ return FetchPlanningResultResponse{}, err
+ }
+
+ path, err := r.scanPlanningPath(endpointFetchPlanResult, ident, planID)
+ if err != nil {
+ return FetchPlanningResultResponse{}, err
+ }
+
+ headers, err := scanPlanningHeaders(nil, opts.AccessDelegation, false)
+ if err != nil {
+ return FetchPlanningResultResponse{}, err
+ }
+
+ resp, err := doGet[FetchPlanningResultResponse](
+ ctx, r.baseURI, path, r.cl,
+ map[int]error{http.StatusNotFound: ErrPlanExpired},
withHeaders(headers))
Review Comment:
Addressed. `FetchPlanningResult` now inspects the REST response’s
`error.type` and maps:
- `NoSuchPlanIdException` → `ErrPlanExpired`
- `NoSuchTableException` → `catalog.ErrNoSuchTable`
- `NoSuchNamespaceException` → `catalog.ErrNoSuchNamespace`
Bare or unrecognized 404s remain `ErrRESTError` rather than being guessed.
Added `httptest` coverage for all mappings and fallback cases.
--
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]
