zhengxiang378928908-code commented on PR #18361: URL: https://github.com/apache/iceberg/pull/18361#issuecomment-6631909380
Thanks, confirmed at `0d68e310b`. I ran `TestMergeAppend` on JDK 21 in three states: | `ManifestWriter` / `PartitionData` | The 4 new delete-file tests (18 cases) | |---|---| | PR head (`0d68e310b`) | all pass | | `ManifestWriter` from `b9aa4ab9b^` (data-file normalization only) | all 18 fail; no other failures in the class | | both reverted to the merge base (`e58bb8aae`) | all 18 fail (44 failures in the class in total) | The data-file-only run is the one that matters here: every delete case fails with `ClassCastException: Integer cannot be cast to Long` from `ManifestWriter.addEntry` when the delete is committed under the promoted spec. So the tests now cover the delete-file path added in `b9aa4ab9b`. At the merge base they fail earlier, in `PartitionData.get` during snapshot summary processing. One small non-blocking point: `positionDeleteAndDVBuiltBeforePromotionCommittedAfter` builds a position delete on v2 and a DV on v3+. Those are the same cases as `positionDeleteBuiltBeforePromotionCommittedAfter` (v2) and `deletionVectorBuiltBeforePromotionCommittedAfter` (v3+), with the same assertions, so it could be dropped. LGTM from my side on the delete-file coverage. -- 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]
