Skip to content

Commit a68602c

Browse files
feat(arrow): resolve S3-compatible schemes in ArrowFileSystemFileIO (#703)
`ArrowFileSystemFileIO::ResolvePath` passed the full URI to the wrapped FileSystem's `PathFromUri`, which only accepts that FileSystem's native scheme(`s3://` for `S3FileSystem`). S3-compatible object stores are commonly addressed with other schemes — `s3a`/`s3n`, or vendor schemes such as `gs://` (GCS) and`oss://` (Alibaba OSS) — served by an `arrow::fs::S3FileSystem` configured with an `endpoint_override`. Every read and write of such a location failed with: ``` expected a URI with one of the schemes (s3) but received <scheme>://... ```
1 parent cf50b12 commit a68602c

4 files changed

Lines changed: 59 additions & 5 deletions

File tree

src/iceberg/arrow/arrow_io.cc

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include <arrow/io/interfaces.h>
3131
#include <arrow/result.h>
3232
#include <arrow/status.h>
33+
#include <arrow/util/uri.h>
3334

3435
#include "iceberg/arrow/arrow_io_internal.h"
3536
#include "iceberg/arrow/arrow_io_util.h"
@@ -473,11 +474,26 @@ class ArrowOutputFile : public OutputFile {
473474
} // namespace
474475

475476
Result<std::string> ArrowFileSystemFileIO::ResolvePath(const std::string& file_location) {
476-
if (file_location.find("://") != std::string::npos) {
477-
ICEBERG_ARROW_ASSIGN_OR_RETURN(auto path, arrow_fs_->PathFromUri(file_location));
478-
return path;
477+
const auto pos = file_location.find("://");
478+
if (pos == std::string::npos) {
479+
return file_location;
479480
}
480-
return file_location;
481+
482+
auto path = arrow_fs_->PathFromUri(file_location);
483+
if (path.ok()) {
484+
return std::move(path).ValueOrDie();
485+
}
486+
487+
// Foreign alias (s3a/s3n): validate via Arrow's parser, then percent-decode the
488+
// scheme-less key (substring keeps a Windows drive letter's ':' that host() drops).
489+
if (auto parsed = ::arrow::util::Uri::FromString(file_location); !parsed.ok()) {
490+
const auto& status = parsed.status();
491+
return std::unexpected<Error>{
492+
{.kind = ToErrorKind(status), .message = status.ToString()}};
493+
}
494+
std::string bucket_key = file_location.substr(pos + 3);
495+
bucket_key = bucket_key.substr(0, bucket_key.find_first_of("?#"));
496+
return ::arrow::util::UriUnescape(bucket_key);
481497
}
482498

483499
Result<std::shared_ptr<::arrow::io::RandomAccessFile>> OpenArrowInputStream(

src/iceberg/catalog/rest/rest_file_io.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ Result<BuiltinFileIOKind> DetectBuiltinFileIO(std::string_view location) {
4545
if (scheme == "file") {
4646
return BuiltinFileIOKind::kArrowLocal;
4747
}
48-
if (scheme == "s3") {
48+
if (scheme == "s3" || scheme == "s3a" || scheme == "s3n") {
4949
return BuiltinFileIOKind::kArrowS3;
5050
}
5151

src/iceberg/test/arrow_io_test.cc

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ namespace iceberg {
3737

3838
namespace {
3939

40+
std::string ForeignSchemeUri(std::string local_path) {
41+
std::ranges::replace(local_path, '\\', '/');
42+
return "x-store://" + local_path;
43+
}
44+
4045
struct CloseState {
4146
bool closed = false;
4247
};
@@ -405,6 +410,35 @@ TEST_F(LocalFileIOTest, StdReadKeepsPositionAvailableAtEof) {
405410
EXPECT_THAT(stream->Position(), HasValue(::testing::Eq(3)));
406411
}
407412

413+
TEST_F(LocalFileIOTest, ResolvesForeignSchemeToUnderlyingPath) {
414+
ASSERT_THAT(file_io_->WriteFile(temp_filepath_, "hello world"), IsOk());
415+
416+
auto read_res = file_io_->ReadFile(ForeignSchemeUri(temp_filepath_), std::nullopt);
417+
EXPECT_THAT(read_res, IsOk());
418+
EXPECT_THAT(read_res, HasValue(::testing::Eq("hello world")));
419+
420+
auto with_query = file_io_->ReadFile(ForeignSchemeUri(temp_filepath_) + "?versionId=42",
421+
std::nullopt);
422+
EXPECT_THAT(with_query, IsOk());
423+
EXPECT_THAT(with_query, HasValue(::testing::Eq("hello world")));
424+
}
425+
426+
TEST_F(LocalFileIOTest, PropagatesNonSchemeMismatchUriError) {
427+
auto read_res = file_io_->ReadFile("file:///tmp/%ZZ", std::nullopt);
428+
EXPECT_THAT(read_res, IsError(ErrorKind::kUnknownError));
429+
EXPECT_THAT(read_res, HasErrorMessage("Cannot parse URI"));
430+
}
431+
432+
TEST_F(LocalFileIOTest, FallbackDecodesPercentEncodingInKey) {
433+
std::string decoded_path = temp_filepath_ + " x";
434+
ASSERT_THAT(file_io_->WriteFile(decoded_path, "raw"), IsOk());
435+
436+
auto read_res =
437+
file_io_->ReadFile(ForeignSchemeUri(temp_filepath_ + "%20x"), std::nullopt);
438+
EXPECT_THAT(read_res, IsOk());
439+
EXPECT_THAT(read_res, HasValue(::testing::Eq("raw")));
440+
}
441+
408442
TEST(FileIOAdapterTest, InputAdapterRejectsReadsAfterClose) {
409443
auto state = std::make_shared<PermissiveReadState>();
410444
state->data = "abc";

src/iceberg/test/rest_file_io_test.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ class MockFileIO : public FileIO {
4949
TEST(RestFileIOTest, DetectBuiltinKindFromScheme) {
5050
EXPECT_THAT(DetectBuiltinFileIO("s3://bucket/path"),
5151
HasValue(::testing::Eq(BuiltinFileIOKind::kArrowS3)));
52+
EXPECT_THAT(DetectBuiltinFileIO("s3a://bucket/path"),
53+
HasValue(::testing::Eq(BuiltinFileIOKind::kArrowS3)));
54+
EXPECT_THAT(DetectBuiltinFileIO("s3n://bucket/path"),
55+
HasValue(::testing::Eq(BuiltinFileIOKind::kArrowS3)));
5256
EXPECT_THAT(DetectBuiltinFileIO("/tmp/warehouse"),
5357
HasValue(::testing::Eq(BuiltinFileIOKind::kArrowLocal)));
5458
EXPECT_THAT(DetectBuiltinFileIO("file:///tmp/warehouse"),

0 commit comments

Comments
 (0)