Skip to content

feat(rest): implement scan planning client methods#1324

Open
Revanth14 wants to merge 1 commit into
apache:mainfrom
Revanth14:feat/rest-scan-planning-client
Open

feat(rest): implement scan planning client methods#1324
Revanth14 wants to merge 1 commit into
apache:mainfrom
Revanth14:feat/rest-scan-planning-client

Conversation

@Revanth14

Copy link
Copy Markdown
Contributor

Summary

This PR implements low-level REST client support for server-side scan planning.

It adds scan-planning endpoint constants, capability checks, request/header plumbing, and the four low-level client methods:

  • PlanTableScan
  • FetchPlanningResult
  • CancelPlanning
  • FetchScanTasks

This is intentionally scoped to the client-method layer only. Higher-level scan orchestration is left for follow-up work.

What Changed

Endpoint Negotiation

Added constants for the scan-planning REST operations:

  • POST /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan
  • GET /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan/{plan-id}
  • DELETE /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan/{plan-id}
  • POST /v1/{prefix}/namespaces/{namespace}/tables/{table}/tasks

These are included in allEndpoints for template validation, but deliberately not in defaultEndpoints. Catalogs must explicitly advertise scan-planning support.

Capability Checks

Implemented:

  • SupportsPlanTableScan
  • SupportsFullRemoteScanPlanning
  • SupportsRemoteScanPlanning

SupportsFullRemoteScanPlanning requires all four endpoints because an initial plan may return async or fanout handles that require polling and task fetch support.

Request/Header Handling

Added per-request support for:

  • Idempotency-Key
  • X-Iceberg-Access-Delegation

PlanTableScan always sends an idempotency key and uses either a per-request access-delegation value or the session default.

FetchPlanningResult uses a per-request access-delegation value when provided, otherwise the session default.

FetchScanTasks sends an idempotency key and suppresses access delegation.

CancelPlanning suppresses access delegation and intentionally defers cancel-specific idempotency/access-delegation API shape until an options type is added.

Shared REST Plumbing

Generalized internal REST helpers so requests can:

  • set per-request headers
  • suppress selected session-default headers
  • allow HTTP 204 No Content

sessionTransport.RoundTrip now treats any header already present on a request as overriding the session default. This is needed so scan-planning calls can override or suppress the default X-Iceberg-Access-Delegation: vended-credentials header.

Response Validation

Preserved PlanTableScanResponse.UnmarshalJSON validation.

Added validation for FetchPlanningResultResponse:

  • failed responses must include an error payload
  • unknown statuses are rejected
  • completed, submitted, and cancelled are accepted for polling

A failed PlanTableScan response still decodes as (resp, nil) when the failed arm contains an error payload; callers must branch on resp.Status.

Out Of Scope

This PR does not implement:

  • WaitForPlan
  • Catalog.PlanFiles
  • scanner delegation
  • expression JSON public wrappers
  • file/delete task decoding
  • residual decoding
  • plan-scoped FileIO wiring
  • retry orchestration

Tests

Added focused tests for endpoint rendering, capability checks, endpoint gating, request headers, default/override access delegation behavior, idempotency key behavior, response validation, ErrPlanExpired mapping, and request-helper body closing.

Validated with:

go test ./catalog/rest
go test ./...

@Revanth14 Revanth14 requested a review from zeroshade as a code owner June 26, 2026 16:52
@Revanth14

Copy link
Copy Markdown
Contributor Author

@laskoviymishka Please take a look when you got time.

@zeroshade zeroshade left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of non-blocking documentation nits from the scan-planning client methods (comment-only, not requesting changes; the low-level surface looks spec-compliant otherwise):

Stale file header - catalog/rest/scan_planning.go lines 17-24: the top-of-file comment still says the method bodies are intentionally unimplemented stubs (returning ErrNotImplemented) and that endpoint capability discovery lands separately in the Phase 0 PR. This PR implements PlanTableScan / FetchPlanningResult / CancelPlanning / FetchScanTasks plus the Supports* capability checks, so that header is now stale and worth updating or removing. (Noting it here rather than inline because those lines aren't part of the diff.)

// cancellation using a detached context with a short timeout.
// cancellation using a detached context with a short timeout. The spec supports
// idempotency and access-delegation headers on cancel; this low-level method
// deliberately defers those until a cancel options type is added, and suppresses

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: per the REST spec, cancelPlanning does accept an Idempotency-Key (the idempotency-key parameter on the DELETE in rest-catalog-open-api.yaml), so deferring it here is reasonable - just flagging so the follow-up that introduces the cancel options type doesn't lose it. The best-effort cancel + 204 handling and access-delegation suppression here look correct.

@Revanth14 Revanth14 force-pushed the feat/rest-scan-planning-client branch from e0d6dd4 to 653fd5f Compare June 26, 2026 18:07
@Revanth14

Copy link
Copy Markdown
Contributor Author

A couple of non-blocking documentation nits from the scan-planning client methods (comment-only, not requesting changes; the low-level surface looks spec-compliant otherwise):

Stale file header - catalog/rest/scan_planning.go lines 17-24: the top-of-file comment still says the method bodies are intentionally unimplemented stubs (returning ErrNotImplemented) and that endpoint capability discovery lands separately in the Phase 0 PR. This PR implements PlanTableScan / FetchPlanningResult / CancelPlanning / FetchScanTasks plus the Supports* capability checks, so that header is now stale and worth updating or removing. (Noting it here rather than inline because those lines aren't part of the diff.)

Updated the stale scan_planning.go file header to reflect the implemented low-level client methods and remaining follow-up scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants