rdblue commented on PR #18546: URL: https://github.com/apache/iceberg/pull/18546#issuecomment-6304221391
@dramaticlly and @stevenzwu, I can explain a bit more about the rationale for changing this. Anoop is working on an API for validations. Right now, we have quite a few methods in `MergingSnapshotProducer` that were added over time as we needed more validations. The idea for v4 is to introduce an API that we can configure so that it can validate v3 or earlier and v4 or later using different logic. For example, v4 is going to be different because validating there are no new deletes for a given data file requires finding the data file and checking its DV matches what was expected. In v3, we have to go through all new deletes, build indexes, and run data file matching. In building an API for validations, we want to simplify where possible. Right now, each validation method like `validateNoNewDeletesForDataFiles` passes all of the requirements in, including `startingSnapshotId` and `conflictDetectionFilter`. Because the filter could differ across calls, the validator can't really cache. But if we update the API to pass those things just once, then it can cache decisions about snapshots and other work. That led to this PR because this is the only place where there are multiple expressions passed as the conflict detection filter. If we can simplify this then we unlock a better API. -- 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]
