Skip to content

Commit ef6c505

Browse files
address review: percent-decode s3a/s3n fallback to match native s3 parsing
1 parent 5c7656b commit ef6c505

2 files changed

Lines changed: 9 additions & 6 deletions

File tree

‎src/iceberg/arrow/arrow_io.cc‎

Lines changed: 4 additions & 2 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"
@@ -490,9 +491,10 @@ Result<std::string> ArrowFileSystemFileIO::ResolvePath(const std::string& file_l
490491
return std::unexpected<Error>{
491492
{.kind = ToErrorKind(status), .message = status.ToString()}};
492493
}
493-
// Scheme-less bucket/key, dropping any ?query / #fragment.
494+
// Scheme-less bucket/key: drop ?query / #fragment and percent-decode to match s3://.
494495
std::string bucket_key = file_location.substr(pos + 3);
495-
return bucket_key.substr(0, bucket_key.find_first_of("?#"));
496+
bucket_key = bucket_key.substr(0, bucket_key.find_first_of("?#"));
497+
return ::arrow::util::UriUnescape(bucket_key);
496498
}
497499

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

‎src/iceberg/test/arrow_io_test.cc‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -429,11 +429,12 @@ TEST_F(LocalFileIOTest, PropagatesNonSchemeMismatchUriError) {
429429
EXPECT_THAT(read_res, HasErrorMessage("Cannot parse URI"));
430430
}
431431

432-
TEST_F(LocalFileIOTest, FallbackPreservesPercentEncodingInKey) {
433-
std::string encoded_path = temp_filepath_ + "%20x";
434-
ASSERT_THAT(file_io_->WriteFile(encoded_path, "raw"), IsOk());
432+
TEST_F(LocalFileIOTest, FallbackDecodesPercentEncodingInKey) {
433+
std::string decoded_path = temp_filepath_ + " x";
434+
ASSERT_THAT(file_io_->WriteFile(decoded_path, "raw"), IsOk());
435435

436-
auto read_res = file_io_->ReadFile(ForeignSchemeUri(encoded_path), std::nullopt);
436+
auto read_res =
437+
file_io_->ReadFile(ForeignSchemeUri(temp_filepath_ + "%20x"), std::nullopt);
437438
EXPECT_THAT(read_res, IsOk());
438439
EXPECT_THAT(read_res, HasValue(::testing::Eq("raw")));
439440
}

0 commit comments

Comments
 (0)