wombatu-kun opened a new pull request, #16910: URL: https://github.com/apache/iceberg/pull/16910
Closes #16905 ## Problem `RewriteTablePathUtil.rewriteManifestList` rewrote each manifest-list entry updating only the manifest path and left `manifest_length` (spec field 501) at the source value. When the target prefix differs in length from the source, the rewritten manifest embeds different-length data-file paths and therefore differs in byte size, so `manifest_length` no longer matches the rewritten manifest on disk. Readers that validate the field (Trino, Impala, iceberg-rust) fail with `Incorrect file size ... (end of stream not reached)`; Spark ignores the field, which is why the bug only surfaces on some engines. ## Fix The rewritten manifest's true length is only known after the manifests are rewritten, but the Spark action wrote the manifest list first. `rebuildMetadata` now discovers the manifests to rewrite (read-only), rewrites them while capturing each one's byte length, and writes the manifest lists last, stamping `manifest_length` from the captured lengths. The length is taken from the manifest writer (`ManifestWriter.length()`) rather than a `getLength()`/HEAD call, so it stays accurate even where the length of an in-progress write underreports. Manifests not rewritten in an incremental run keep their original length. Because the staged metadata is copied to the target byte-for-byte (Distcp), the recorded length equals the target file's actual size. ## Tests - `TestRewriteTablePathsAction#testManifestLengthAfterRewrite`: rewrites a table to a longer target prefix and asserts every rewritten manifest-list entry's `manifest_length` matches the on-disk manifest size (format versions 2-4, covering both Avro and Parquet manifests). Fails before this change, passes after. - `TestRewriteTablePathUtil`: unit coverage for stamping `manifest_length` from the length map and for falling back to the source length when a manifest is absent from the map. ## Notes Same class of bug as #15470 (delete-file `file_size_in_bytes` inside manifests), but for `manifest_length` in the manifest list; the two fixes are independent. They touch overlapping code in the rewrite path, so whichever lands second will need a small rebase. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
