From 4ddbbdb15ca7ea1f51aeb206d44e8399d97c7b31 Mon Sep 17 00:00:00 2001 From: alytantawyy Date: Fri, 12 Jun 2026 13:42:55 -0700 Subject: [PATCH 1/2] GH-50148: [C++] Add Content-Encoding support to S3 filesystem metadata --- cpp/src/arrow/filesystem/s3fs.cc | 9 +++++++++ cpp/src/arrow/filesystem/s3fs_test.cc | 10 ++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/filesystem/s3fs.cc b/cpp/src/arrow/filesystem/s3fs.cc index 1c6763a4aee9..a93f6054073d 100644 --- a/cpp/src/arrow/filesystem/s3fs.cc +++ b/cpp/src/arrow/filesystem/s3fs.cc @@ -1361,6 +1361,7 @@ std::shared_ptr GetObjectMetadata(const ObjectResult& re md->Append("Content-Length", ToChars(result.GetContentLength())); push("Cache-Control", result.GetCacheControl()); + push("Content-Encoding", result.GetContentEncoding()); push("Content-Type", result.GetContentType()); push("Content-Language", result.GetContentLanguage()); push("ETag", result.GetETag()); @@ -1379,6 +1380,7 @@ struct ObjectMetadataSetter { static std::unordered_map GetSetters() { return {{"ACL", CannedACLSetter()}, {"Cache-Control", StringSetter(&ObjectRequest::SetCacheControl)}, + {"Content-Encoding", ContentEncodingSetter()}, {"Content-Type", ContentTypeSetter()}, {"Content-Language", StringSetter(&ObjectRequest::SetContentLanguage)}, {"Expires", DateTimeSetter(&ObjectRequest::SetExpires)}}; @@ -1419,6 +1421,13 @@ struct ObjectMetadataSetter { }; } + static Setter ContentEncodingSetter() { + return [](const std::string& str, ObjectRequest* req) { + req->SetContentEncoding(str); + return Status::OK(); + }; + } + static Result ParseACL(const std::string& v) { if (v.empty()) { return S3Model::ObjectCannedACL::NOT_SET; diff --git a/cpp/src/arrow/filesystem/s3fs_test.cc b/cpp/src/arrow/filesystem/s3fs_test.cc index 114701b6d446..56b143843b9e 100644 --- a/cpp/src/arrow/filesystem/s3fs_test.cc +++ b/cpp/src/arrow/filesystem/s3fs_test.cc @@ -1655,8 +1655,9 @@ TEST_F(TestS3FS, OpenOutputStreamMetadata) { testing::IsSupersetOf(implicit_metadata->sorted_pairs())); // Create new file with explicit metadata - auto metadata = KeyValueMetadata::Make({"Content-Type", "Expires"}, - {"x-arrow/test6", "2016-02-05T20:08:35Z"}); + auto metadata = KeyValueMetadata::Make( + {"Content-Encoding", "Content-Type", "Expires"}, + {"gzip", "x-arrow/test6", "2016-02-05T20:08:35Z"}); AssertMetadataRoundtrip("bucket/mdfile1", metadata, testing::IsSupersetOf(metadata->sorted_pairs())); @@ -1666,8 +1667,9 @@ TEST_F(TestS3FS, OpenOutputStreamMetadata) { AssertMetadataRoundtrip("bucket/mdfile2", metadata, testing::_); // Create new file with default metadata - auto default_metadata = KeyValueMetadata::Make({"Content-Type", "Content-Language"}, - {"image/png", "fr_FR"}); + auto default_metadata = KeyValueMetadata::Make( + {"Content-Encoding", "Content-Type", "Content-Language"}, + {"br", "image/png", "fr_FR"}); options_.default_metadata = default_metadata; MakeFileSystem(); // (null, then empty metadata argument) From d2ed7e2b69ca7ea6b948fff9baf8d02d5d364612 Mon Sep 17 00:00:00 2001 From: alytantawyy Date: Mon, 28 Sep 2026 20:58:26 -0700 Subject: [PATCH 2/2] GH-50148: [C++] Address S3 metadata review feedback --- cpp/src/arrow/filesystem/s3fs.cc | 9 +-------- cpp/src/arrow/filesystem/s3fs_test.cc | 12 ++++++------ 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/cpp/src/arrow/filesystem/s3fs.cc b/cpp/src/arrow/filesystem/s3fs.cc index a93f6054073d..76b1ce616143 100644 --- a/cpp/src/arrow/filesystem/s3fs.cc +++ b/cpp/src/arrow/filesystem/s3fs.cc @@ -1380,7 +1380,7 @@ struct ObjectMetadataSetter { static std::unordered_map GetSetters() { return {{"ACL", CannedACLSetter()}, {"Cache-Control", StringSetter(&ObjectRequest::SetCacheControl)}, - {"Content-Encoding", ContentEncodingSetter()}, + {"Content-Encoding", StringSetter(&ObjectRequest::SetContentEncoding)}, {"Content-Type", ContentTypeSetter()}, {"Content-Language", StringSetter(&ObjectRequest::SetContentLanguage)}, {"Expires", DateTimeSetter(&ObjectRequest::SetExpires)}}; @@ -1421,13 +1421,6 @@ struct ObjectMetadataSetter { }; } - static Setter ContentEncodingSetter() { - return [](const std::string& str, ObjectRequest* req) { - req->SetContentEncoding(str); - return Status::OK(); - }; - } - static Result ParseACL(const std::string& v) { if (v.empty()) { return S3Model::ObjectCannedACL::NOT_SET; diff --git a/cpp/src/arrow/filesystem/s3fs_test.cc b/cpp/src/arrow/filesystem/s3fs_test.cc index 56b143843b9e..06d25287a4f3 100644 --- a/cpp/src/arrow/filesystem/s3fs_test.cc +++ b/cpp/src/arrow/filesystem/s3fs_test.cc @@ -1655,9 +1655,9 @@ TEST_F(TestS3FS, OpenOutputStreamMetadata) { testing::IsSupersetOf(implicit_metadata->sorted_pairs())); // Create new file with explicit metadata - auto metadata = KeyValueMetadata::Make( - {"Content-Encoding", "Content-Type", "Expires"}, - {"gzip", "x-arrow/test6", "2016-02-05T20:08:35Z"}); + auto metadata = + KeyValueMetadata::Make({"Content-Encoding", "Content-Type", "Expires"}, + {"gzip", "x-arrow/test6", "2016-02-05T20:08:35Z"}); AssertMetadataRoundtrip("bucket/mdfile1", metadata, testing::IsSupersetOf(metadata->sorted_pairs())); @@ -1667,9 +1667,9 @@ TEST_F(TestS3FS, OpenOutputStreamMetadata) { AssertMetadataRoundtrip("bucket/mdfile2", metadata, testing::_); // Create new file with default metadata - auto default_metadata = KeyValueMetadata::Make( - {"Content-Encoding", "Content-Type", "Content-Language"}, - {"br", "image/png", "fr_FR"}); + auto default_metadata = + KeyValueMetadata::Make({"Content-Encoding", "Content-Type", "Content-Language"}, + {"br", "image/png", "fr_FR"}); options_.default_metadata = default_metadata; MakeFileSystem(); // (null, then empty metadata argument)