jopdorp commented on PR #3564: URL: https://github.com/apache/iceberg-rust/pull/3564#issuecomment-6683355768
@CTTY #3300 (overwrite) sits on this branch now as well, it only builds a `MergingIntent` with `fail_missing_delete_paths` on. Three things from putting it on top, none blocking for us: 1. Missing delete paths are checked after the loop in `ManifestFilterManager::filter` has written its rewritten manifests, and `validate_added_files` also runs after the filters wrote. On #3300 @laskoviymishka asked for everything to be matched before the first write. They land in `owned_artifacts`, so #3417 covers them and maybe that is enough. 2. The `MergingIntent` doc says the action validates disjoint add/remove sets. Overwrite allows deleting a path and re-adding it in the same commit, which was asked for in #3300's review, and the producer handles it fine: a deleted entry plus an added one. Should the doc allow that, or do you want actions to reject it? 3. On v3, should a data file's deletion vectors go when the data file is removed? Overwrite leaves `removed_delete_files` empty, so they stay live. I have not checked what Java does here. Happy to send a PR into your branch for 1 if you want it. -- 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]
