Revanth14 commented on PR #2099: URL: https://github.com/apache/iceberg-go/pull/2099#issuecomment-5966842887
> Registering the existing `validateNoNewDeletesForRewrittenFiles` through `addValidator` for every data file the copy-on-write path removes closes #2090 and matches Java's copy-on-write `validateNoConflictingDeletes()` at both isolation levels. I confirmed that the new conflict cases fail without the fix (the replay brings `id=4` back) and pass here. > > ### Smaller observations > * The append control can't catch an over-broad validator; see the inline comment for a v3 case that can. > * On v2, the partition fallback fires for every position delete iceberg-go itself writes. `referenced_data_file` is unset, and the default `truncate(16)` metrics clip the `file_path` bounds. So a concurrent MoR delete on a _different_ file in the same partition (anywhere, if the table is unpartitioned) also rejects a retried CoW commit; a probe confirmed this. It's safe and your description mentions the fallback. Java's writer, though, keeps exact `file_path` bounds (`MetricsConfig.forPositionDelete()`), so its fallback rarely fires. That's worth a writer-side follow-up rather than a change here. > * Heads-up: #2046 turns `filesToRewrite` into `[]iceberg.ManifestEntry`, so whichever PR lands second needs to adapt the `removed` slice. > * The commit is missing the `Signed-off-by` trailer that CONTRIBUTING.md asks for. > > > _This review was drafted by an AI-assisted tool and > > confirmed by an Apache Iceberg maintainer. The maintainer > > approving this PR has read the findings and signed off. If > > something feels off, please reply on the PR and a maintainer > > will follow up._ > > _More on how Apache Iceberg handles maintainer review:_ > > [CONTRIBUTING.md](https://github.com/apache/iceberg-go/blob/main/CONTRIBUTING.md). Thanks for the review! applied the changes. -- 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]
