Repository navigation
fix(metadata): write and detect gzip-compressed table metadata - #1014
LuciferYang wants to merge 5 commits into
Conversation
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.
|
|
||
| Result<MetadataFileCodecType> TableMetadataUtil::Codec::FromFileName( | ||
| std::string_view file_name) { | ||
| auto pos = file_name.find_last_of(kTableMetadataFileSuffix); |
There was a problem hiding this comment.
Good catch! Looks like this was wrong :)
| return {}; | ||
| } | ||
|
|
||
| Result<std::string> Compress(const std::string& data) { |
There was a problem hiding this comment.
No deflateReset() here, so a second Compress() on the same GZipCompressor fails once the first hits Z_STREAM_END. Only latent today since Write() builds a fresh one, but the Init()/Compress() split reads as reusable.
There was a problem hiding this comment.
Bumping this - latent only because Write() builds a fresh compressor each time, but the Init()/Compress() split invites reuse.
There was a problem hiding this comment.
Fixed in 68c573a: Compress() now calls deflateReset() once the stream finishes, so one compressor can be reused. GZipCompressorReused covers it: two payloads through one instance, which failed with Z_BUF_ERROR before the change.
What
With
write.metadata.compression-codec=gzip, metadata files are named<version>-<uuid>.gz.metadata.json, butTableMetadataUtil::Writestored plain JSON in them, andCodec::FromFileNamemisdetected the codec:find_last_of(".metadata.json")searches for any one character of that string, so a.gz.metadata.jsonname was classified as uncompressed. The two bugs cancelled out inside iceberg-cpp, while files exchanged with Java failed in both directions.Closes #1013.
How
FromFileNameusesrfind, matching Java'slastIndexOf(".metadata.json"). Names without.metadata.jsonare now rejected, as in Java.Writecompresses the JSON when the location's codec is gzip, matching Java'sinternalWrite. This adds aGZipCompressornext toGZipDecompressorinutil/gzip_internal.h, plus akCompressErrorerror kind alongsidekDecompressError. Inputs above zlib's 32-bit length limit are rejected instead of being silently truncated.The two changes are in one PR because neither works alone: fixing only the read side would make iceberg-cpp try to inflate the plain JSON it writes today.
Tables that already have
write.metadata.compression-codec=gzipand were written by iceberg-cpp contain plain JSON under.gz.metadata.jsonnames; after this change iceberg-cpp reads those as gzip and fails, as Java already does. The property defaults tonone.Testing
New tests in
metadata_io_test.cc:CodecFromFileGzMetadataJson,CodecFromFilePlainAndLegacyGz,CodecFromFileInvalidName: codec detection for the.gz.metadata.json, plain, legacy.metadata.json.gz, and invalid names.RoundTripGzipCompressedMetadata: writes with the gzip codec, checks the file name, checks that the bytes on disk start with the gzip magic1f 8b, and reads the metadata back.New tests in
gzip_decompress_test.cc:GZipCompressorRoundTrip(100 KiB, more than one output buffer),GZipCompressorEmptyInput, andGZipCompressorReused(one compressor, two payloads, each decompresses on its own).Reverting the
rfindchange fails the three codec tests and the round trip; removing the write-side compression fails the round trip.