hedger9487 commented on PR #3631: URL: https://github.com/apache/iceberg-python/pull/3631#issuecomment-5521065380
Hi @moomindani, Thanks for taking the initiative on this feature and setting up a great foundation! While working on the follow-up for #3840 (suggested by @kevinjqliu to allow rewriting legacy manifests), I came across your PR. Based on the Java Iceberg reference (`BaseRewriteManifests`) and #3840's requirements, I noticed a few additional capabilities that might be useful: 1. **Single manifest rewriting**: Currently, `if len(group) == 1: continue` skips rewriting single manifests. For use cases like #3840 (upgrading legacy manifest schemas to standard types), users need to be able to rewrite/upgrade single manifests as well. 2. **Selective filtering (`rewrite_if`)**: Adding `rewrite_if(predicate)` from Java's `RewriteManifests.rewriteIf()` allows users to selectively target specific manifests (e.g. only legacy manifests or specific partitions). 3. **Bin-packing & Concurrency**: Reusing PyIceberg's `ListPacker` and `ExecutorFactory` allows parallel manifest writes for large tables. 4. **Safety guard**: Adding `_validate_files_counts` ensures zero data files are dropped during the rewrite. I already have a tested implementation with these enhancements ready locally. If you're open to it, I'd be more than happy to collaborate by opening a PR directly to your branch or co-authoring to help land this together! Thanks again for your great work! -- 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]
