Skip to content

Commit 5107eda

Browse files
authored
fix(metadata): write and detect gzip-compressed table metadata (#1014)
Codec::FromFileName used find_last_of with the multi-character suffix '.metadata.json', which matches the last single character of the set (the final 'n') instead of the suffix. For gzip-compressed metadata written by Iceberg Java as '<name>.gz.metadata.json', the resulting file_name_without_suffix kept the wrong tail, the '.gz' check failed, and the file was misclassified as uncompressed kNone. Reading such a file then fails to decompress. Use rfind so the whole suffix matches. The legacy '.metadata.json.gz' compatibility branch is unchanged. Round-trip reading of Java-written gzip metadata files is covered by new unit tests. NewTableMetadataFilePath names the file '<version>-<uuid>.gz.metadata.json' when write.metadata.compression-codec is gzip, but Write always emitted plain JSON, so the file contradicted its own name and no reader could inflate it. Add a GZipCompressor next to the existing GZipDecompressor and compress in Write when the location's codec is gzip, completing the round trip.
1 parent 29900ca commit 5107eda

6 files changed

Lines changed: 193 additions & 1 deletion

File tree

‎src/iceberg/result.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ enum class ErrorKind {
3939
kBadRequest,
4040
kCommitFailed,
4141
kCommitStateUnknown,
42+
kCompressError,
4243
kDecompressError,
4344
kForbidden,
4445
kInternalServerError,
@@ -106,6 +107,7 @@ DEFINE_ERROR_FUNCTION(AuthenticationFailed)
106107
DEFINE_ERROR_FUNCTION(BadRequest)
107108
DEFINE_ERROR_FUNCTION(CommitFailed)
108109
DEFINE_ERROR_FUNCTION(CommitStateUnknown)
110+
DEFINE_ERROR_FUNCTION(CompressError)
109111
DEFINE_ERROR_FUNCTION(DecompressError)
110112
DEFINE_ERROR_FUNCTION(Forbidden)
111113
DEFINE_ERROR_FUNCTION(InternalServerError)

‎src/iceberg/table_metadata.cc‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ Result<MetadataFileCodecType> TableMetadataUtil::Codec::FromString(
425425

426426
Result<MetadataFileCodecType> TableMetadataUtil::Codec::FromFileName(
427427
std::string_view file_name) {
428-
auto pos = file_name.find_last_of(kTableMetadataFileSuffix);
428+
auto pos = file_name.rfind(kTableMetadataFileSuffix);
429429
if (pos == std::string::npos) {
430430
return InvalidArgument("{} is not a valid metadata file", file_name);
431431
}
@@ -489,8 +489,16 @@ Result<std::string> TableMetadataUtil::Write(FileIO& io, const TableMetadata* ba
489489

490490
Status TableMetadataUtil::Write(FileIO& io, const std::string& location,
491491
const TableMetadata& metadata) {
492+
ICEBERG_ASSIGN_OR_RAISE(auto codec_type, Codec::FromFileName(location));
492493
ICEBERG_ASSIGN_OR_RAISE(auto json, ToJson(metadata));
493494
ICEBERG_ASSIGN_OR_RAISE(auto json_string, ToJsonString(json));
495+
496+
if (codec_type == MetadataFileCodecType::kGzip) {
497+
auto gzip_compressor = std::make_unique<GZipCompressor>();
498+
ICEBERG_RETURN_UNEXPECTED(gzip_compressor->Init());
499+
ICEBERG_ASSIGN_OR_RAISE(json_string, gzip_compressor->Compress(json_string));
500+
}
501+
494502
return io.WriteFile(location, json_string);
495503
}
496504

‎src/iceberg/test/gzip_decompress_test.cc‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,4 +84,53 @@ TEST_F(GZipTest, GZipDecompressedString) {
8484
ASSERT_EQ(read_data.value(), test_string);
8585
}
8686

87+
TEST_F(GZipTest, GZipCompressorRoundTrip) {
88+
// Larger than the 32 KiB output buffer, so compression needs several passes.
89+
auto test_string = GenerateRandomString(100 * 1024);
90+
91+
GZipCompressor compressor;
92+
ASSERT_THAT(compressor.Init(), IsOk());
93+
ICEBERG_UNWRAP_OR_FAIL(auto compressed, compressor.Compress(test_string));
94+
ASSERT_GE(compressed.size(), 2u);
95+
EXPECT_EQ(static_cast<unsigned char>(compressed[0]), 0x1f);
96+
EXPECT_EQ(static_cast<unsigned char>(compressed[1]), 0x8b);
97+
98+
GZipDecompressor decompressor;
99+
ASSERT_THAT(decompressor.Init(), IsOk());
100+
ICEBERG_UNWRAP_OR_FAIL(auto decompressed, decompressor.Decompress(compressed));
101+
EXPECT_EQ(decompressed, test_string);
102+
}
103+
104+
TEST_F(GZipTest, GZipCompressorEmptyInput) {
105+
GZipCompressor compressor;
106+
ASSERT_THAT(compressor.Init(), IsOk());
107+
ICEBERG_UNWRAP_OR_FAIL(auto compressed, compressor.Compress(""));
108+
// An empty payload still yields a complete gzip stream (header and trailer).
109+
EXPECT_FALSE(compressed.empty());
110+
111+
GZipDecompressor decompressor;
112+
ASSERT_THAT(decompressor.Init(), IsOk());
113+
ICEBERG_UNWRAP_OR_FAIL(auto decompressed, decompressor.Decompress(compressed));
114+
EXPECT_TRUE(decompressed.empty());
115+
}
116+
117+
TEST_F(GZipTest, GZipCompressorReused) {
118+
// One compressor can compress several payloads; each call yields a complete,
119+
// independent gzip stream.
120+
GZipCompressor compressor;
121+
ASSERT_THAT(compressor.Init(), IsOk());
122+
123+
for (const auto& payload : {GenerateRandomString(1024), GenerateRandomString(2048)}) {
124+
ICEBERG_UNWRAP_OR_FAIL(auto compressed, compressor.Compress(payload));
125+
ASSERT_GE(compressed.size(), 2u);
126+
EXPECT_EQ(static_cast<unsigned char>(compressed[0]), 0x1f);
127+
EXPECT_EQ(static_cast<unsigned char>(compressed[1]), 0x8b);
128+
129+
GZipDecompressor decompressor;
130+
ASSERT_THAT(decompressor.Init(), IsOk());
131+
ICEBERG_UNWRAP_OR_FAIL(auto decompressed, decompressor.Decompress(compressed));
132+
EXPECT_EQ(decompressed, payload);
133+
}
134+
}
135+
87136
} // namespace iceberg

‎src/iceberg/test/metadata_io_test.cc‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,53 @@ TEST_F(MetadataIOTest, ReadWriteCompressedMetadata) {
138138
EXPECT_EQ(*metadata_read, metadata);
139139
}
140140

141+
TEST_F(MetadataIOTest, CodecFromFileGzMetadataJson) {
142+
// Java writers emit gzip-compressed metadata as "<name>.gz.metadata.json";
143+
// the codec must be detected from the ".gz" before the metadata suffix.
144+
auto result = TableMetadataUtil::Codec::FromFileName("00001-abc.gz.metadata.json");
145+
ASSERT_TRUE(result.has_value());
146+
EXPECT_EQ(*result, MetadataFileCodecType::kGzip);
147+
}
148+
149+
TEST_F(MetadataIOTest, CodecFromFilePlainAndLegacyGz) {
150+
auto plain = TableMetadataUtil::Codec::FromFileName("00001-abc.metadata.json");
151+
ASSERT_TRUE(plain.has_value());
152+
EXPECT_EQ(*plain, MetadataFileCodecType::kNone);
153+
154+
auto legacy_gz = TableMetadataUtil::Codec::FromFileName("00001-abc.metadata.json.gz");
155+
ASSERT_TRUE(legacy_gz.has_value());
156+
EXPECT_EQ(*legacy_gz, MetadataFileCodecType::kGzip);
157+
}
158+
159+
TEST_F(MetadataIOTest, CodecFromFileInvalidName) {
160+
// Matches Iceberg Java fromFileName: a name without the '.metadata.json'
161+
// substring is rejected as not a valid metadata file.
162+
EXPECT_THAT(TableMetadataUtil::Codec::FromFileName("foo.txt"),
163+
IsError(ErrorKind::kInvalidArgument));
164+
}
165+
166+
TEST_F(MetadataIOTest, RoundTripGzipCompressedMetadata) {
167+
// With metadata compression enabled the writer names the file
168+
// "<version>-<uuid>.gz.metadata.json", the same layout Iceberg Java uses.
169+
// The payload must really be gzip, and reading it back must decompress it.
170+
TableMetadata metadata = PrepareMetadata();
171+
metadata.properties.Set(TableProperties::kMetadataCompression, std::string("gzip"));
172+
173+
ICEBERG_UNWRAP_OR_FAIL(auto written_location,
174+
TableMetadataUtil::Write(*io_, nullptr, "", metadata));
175+
EXPECT_THAT(written_location, testing::HasSubstr(".gz.metadata.json"));
176+
177+
// The bytes on disk carry the gzip magic, so another engine can read them.
178+
ICEBERG_UNWRAP_OR_FAIL(auto raw, io_->ReadFile(written_location, std::nullopt));
179+
ASSERT_GE(raw.size(), 2u);
180+
EXPECT_EQ(static_cast<unsigned char>(raw[0]), 0x1f);
181+
EXPECT_EQ(static_cast<unsigned char>(raw[1]), 0x8b);
182+
183+
ICEBERG_UNWRAP_OR_FAIL(auto metadata_read,
184+
TableMetadataUtil::Read(*io_, written_location));
185+
EXPECT_EQ(*metadata_read, metadata);
186+
}
187+
141188
TEST_F(MetadataIOTest, WriteMetadataWithBase) {
142189
TableMetadata base = PrepareMetadata();
143190

‎src/iceberg/util/gzip_internal.cc‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <zlib.h>
2424

2525
#include <cstring>
26+
#include <limits>
2627
#include <memory>
2728
#include <string>
2829
#include <vector>
@@ -86,6 +87,65 @@ class ZlibImpl {
8687
z_stream stream_;
8788
};
8889

90+
class ZlibDeflateImpl {
91+
public:
92+
ZlibDeflateImpl() { memset(&stream_, 0, sizeof(stream_)); }
93+
94+
~ZlibDeflateImpl() {
95+
if (initialized_) {
96+
deflateEnd(&stream_);
97+
}
98+
}
99+
100+
Status Init() {
101+
// Maximum window size, plus the offset that selects a gzip wrapper over
102+
// the raw zlib one, so GZipDecompressor's auto-detection accepts it.
103+
constexpr int kWindowBits = 15;
104+
constexpr int kGzipWrapper = 16;
105+
int ret = deflateInit2(&stream_, Z_DEFAULT_COMPRESSION, Z_DEFLATED,
106+
kWindowBits | kGzipWrapper, 8, Z_DEFAULT_STRATEGY);
107+
if (ret != Z_OK) {
108+
return CompressError("deflateInit2 failed, result:{}", ret);
109+
}
110+
initialized_ = true;
111+
return {};
112+
}
113+
114+
Result<std::string> Compress(const std::string& data) {
115+
if (data.size() > std::numeric_limits<uInt>::max()) {
116+
return CompressError("Cannot compress {} bytes in a single gzip pass", data.size());
117+
}
118+
if (!initialized_) {
119+
ICEBERG_RETURN_UNEXPECTED(Init());
120+
}
121+
stream_.avail_in = static_cast<uInt>(data.size());
122+
stream_.next_in = reinterpret_cast<Bytef*>(const_cast<char*>(data.data()));
123+
124+
std::vector<char> out_buffer(32 * 1024);
125+
std::string result;
126+
int ret = 0;
127+
do {
128+
stream_.avail_out = static_cast<uInt>(out_buffer.size());
129+
stream_.next_out = reinterpret_cast<Bytef*>(out_buffer.data());
130+
ret = deflate(&stream_, Z_FINISH);
131+
if (ret != Z_OK && ret != Z_STREAM_END) {
132+
return CompressError("deflate failed, result:{}", ret);
133+
}
134+
result.append(out_buffer.data(), out_buffer.size() - stream_.avail_out);
135+
} while (ret != Z_STREAM_END);
136+
// Rewind the stream so the next Compress() starts a fresh gzip member.
137+
ret = deflateReset(&stream_);
138+
if (ret != Z_OK) {
139+
return CompressError("deflateReset failed, result:{}", ret);
140+
}
141+
return result;
142+
}
143+
144+
private:
145+
bool initialized_ = false;
146+
z_stream stream_;
147+
};
148+
89149
GZipDecompressor::GZipDecompressor() : zlib_impl_(std::make_unique<ZlibImpl>()) {}
90150

91151
GZipDecompressor::~GZipDecompressor() = default;
@@ -96,4 +156,14 @@ Result<std::string> GZipDecompressor::Decompress(const std::string& compressed_d
96156
return zlib_impl_->Decompress(compressed_data);
97157
}
98158

159+
GZipCompressor::GZipCompressor() : zlib_impl_(std::make_unique<ZlibDeflateImpl>()) {}
160+
161+
GZipCompressor::~GZipCompressor() = default;
162+
163+
Status GZipCompressor::Init() { return zlib_impl_->Init(); }
164+
165+
Result<std::string> GZipCompressor::Compress(const std::string& data) {
166+
return zlib_impl_->Compress(data);
167+
}
168+
99169
} // namespace iceberg

‎src/iceberg/util/gzip_internal.h‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
namespace iceberg {
2828

2929
class ZlibImpl;
30+
class ZlibDeflateImpl;
3031

3132
class GZipDecompressor {
3233
public:
@@ -42,4 +43,19 @@ class GZipDecompressor {
4243
std::unique_ptr<ZlibImpl> zlib_impl_;
4344
};
4445

46+
/// \brief Compresses data into the gzip format read by GZipDecompressor.
47+
class GZipCompressor {
48+
public:
49+
GZipCompressor();
50+
51+
~GZipCompressor();
52+
53+
Status Init();
54+
55+
Result<std::string> Compress(const std::string& data);
56+
57+
private:
58+
std::unique_ptr<ZlibDeflateImpl> zlib_impl_;
59+
};
60+
4561
} // namespace iceberg

0 commit comments

Comments
 (0)