adityaksolves commented on issue #18482: URL: https://github.com/apache/iceberg/issues/18482#issuecomment-6150617320
Thanks @manuzhang. I've reproduced both behaviors locally on formats v2, v3 and v4, and I have a fix ready. Will raise a PR shortly. My proposal is to reject v1-format manifests in `RewriteManifests.addManifest` on v2+ tables with a clear error (`Cannot add v1 manifest to a v2 table: <path>`), rather than copying them with a default sequence number of 0. Reasons: - It fails fast at the API call instead of committing a manifest that breaks scan planning later. - A single check at the top of `addManifest` covers both the reused-manifest and copied-manifest paths. - Silently rewriting a v1 manifest into v2+ is a larger behavior change; callers who really want that can rewrite the manifest with a v2+ writer first. The check reads the `format-version` key from the Avro manifest header (absent means v1). Non-Avro manifests are skipped since v1 manifests are always Avro. I'm adding a test in `TestRewriteManifests` that covers the reproduction from the issue. Happy to switch to the "copy with sequence number 0" approach if you or other reviewers prefer that. -- 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]
