Skip to content

feat(io): add ResolvingFileIO to resolve FileIO by location scheme and forward vended credentials#828

Open
plusplusjiajia wants to merge 2 commits into
apache:mainfrom
plusplusjiajia:feat-resolving-fileio
Open

feat(io): add ResolvingFileIO to resolve FileIO by location scheme and forward vended credentials#828
plusplusjiajia wants to merge 2 commits into
apache:mainfrom
plusplusjiajia:feat-resolving-fileio

Conversation

@plusplusjiajia

Copy link
Copy Markdown
Member

Follow-up to #719: replaces the temporary warehouse-based FileIO detection
with a scheme-based ResolvingFileIO, mirroring Java.

Why

The FileIO impl was picked from DetectBuiltinFileIO(warehouse), so any
non-s3:// warehouse resolved to arrow-fs-local and binding vended
credentials failed with Configured FileIO does not support vended storage credentials. Credential-vending catalogs typically use a logical warehouse
identifier (bucket ARN / catalog name), not a storage URI, so the feature
only worked when warehouse was literally s3://….

What

  1. ResolvingFileIO (core) — picks the impl per file-path scheme
    (s3/s3a/s3n/ossarrow-fs-s3; file/no scheme → local; else
    NotSupported), lazily loaded via FileIORegistry and cached; forwards
    the full credential list to impls that support it (Java
    ResolvingFileIO semantics). Registered as resolving-file-io.
  2. REST defaultMakeCatalogFileIO defaults to resolving-file-io
    when io-impl is unset (Java DEFAULT_FILE_IO_IMPL likewise); the
    warehouse detection and the io-impl-vs-warehouse check are removed.
    Explicit io-impl is honored as before.
  3. ArrowS3FileIOSetStorageCredentials skips non-S3 credential
    prefixes instead of rejecting the set (Java S3FileIO filters by "s3");
    servers may vend credentials for several storage systems at once.

@MisterRaindrop

Copy link
Copy Markdown
Contributor

Could we add an end-to-end test using the actual Arrow FileIO implementations? The current tests use mocks and don’t cover the full REST → ResolvingFileIO → FileIORegistry → Arrow FileIO path.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a core ResolvingFileIO that selects the concrete FileIO implementation based on each file path’s URI scheme (instead of inferring from warehouse), and updates the REST catalog defaults and S3 credential handling to match Iceberg Java semantics for vended credentials.

Changes:

  • Add ResolvingFileIO (core) with per-scheme resolution, lazy loading via FileIORegistry, caching, and forwarding of vended storage credentials.
  • Change REST catalog MakeCatalogFileIO default to resolving-file-io when io-impl is unset; remove warehouse-based auto-detection and compatibility checks.
  • Update ArrowS3FileIO::SetStorageCredentials to skip (not reject) non-S3 credential prefixes and warn when none are S3-compatible; add/adjust tests accordingly.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/iceberg/test/rest_file_io_test.cc Updates REST FileIO tests for resolving default and logical-warehouse credential binding regression coverage.
src/iceberg/test/resolving_file_io_test.cc Adds unit tests for scheme resolution, routing/caching behavior, and credential forwarding semantics.
src/iceberg/test/meson.build Adds resolving_file_io_test.cc to Meson test sources.
src/iceberg/test/CMakeLists.txt Adds resolving_file_io_test.cc to CMake test sources.
src/iceberg/test/arrow_s3_file_io_test.cc Updates S3 FileIO credential-prefix behavior tests (skip non-S3; accept no-S3 case).
src/iceberg/resolving_file_io.h Declares new ResolvingFileIO API and caching/credential-forwarding behavior.
src/iceberg/resolving_file_io.cc Implements per-scheme resolution, lazy registry loading, caching, and credential forwarding.
src/iceberg/meson.build Adds resolving_file_io.cc to core build sources and installs the new header.
src/iceberg/file_io_registry.h Introduces the kResolvingFileIO registry name constant.
src/iceberg/file_io_registry.cc Pre-registers resolving-file-io in the registry state constructor.
src/iceberg/CMakeLists.txt Adds resolving_file_io.cc to core CMake sources.
src/iceberg/catalog/rest/rest_file_io.h Removes builtin-detection declarations; documents resolving default behavior.
src/iceberg/catalog/rest/rest_file_io.cc Defaults to resolving-file-io when io-impl is unset; simplifies table FileIO creation flow.
src/iceberg/arrow/s3/arrow_s3_file_io.cc Skips non-S3 credential prefixes and logs a warning when all are skipped.

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.

3 participants