adityaksolves commented on PR #18502: URL: https://github.com/apache/iceberg/pull/18502#issuecomment-6362942710
Hi @amogh-jahagirdar @RussellSpitzer, could one of you take a look when you get a chance? This fixes #18482, where `RewriteManifests.addManifest` mishandles v1-format manifests on v2+ tables (NPE during scan planning on the reused path, `IllegalStateException` on the copied path). One design question where I'd value your input: I went with rejecting v1 manifests up front on v2+ tables with a clear `IllegalArgumentException`, rather than copying them and writing the spec default of `0` as explicit sequence numbers (the approach iceberg-cpp takes in apache/iceberg-cpp#774). My reasoning is that it fails fast and avoids silently upgrading the manifest format, but I'm happy to switch to copy-with-0 if that's preferred for Java. I've rebased onto the latest `main` and CI is green. Thanks! -- 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]
