openinx commented on a change in pull request #3069: URL: https://github.com/apache/iceberg/pull/3069#discussion_r702772776
########## File path: api/src/main/java/org/apache/iceberg/OverwriteFiles.java ########## @@ -122,27 +122,30 @@ * * @param conflictDetectionFilter an expression on rows in the table * @return this for method chaining + * @deprecated this will be removed in 0.14.0; + * use {@link #validateNoConflictingOperations(Expression)} instead */ - OverwriteFiles validateNoConflictingAppends(Expression conflictDetectionFilter); + @Deprecated + default OverwriteFiles validateNoConflictingAppends(Expression conflictDetectionFilter) { + return validateNoConflictingOperations(conflictDetectionFilter); + } /** - * Enables validation that files added concurrently do not conflict with this commit's operation. + * Enables validation that no concurrent operation conflicts with this commit. * <p> * This method should be called when the table is queried to determine which files to delete/append. - * If a concurrent operation commits a new file after the data was read and that file might - * contain rows matching the specified conflict detection filter, the overwrite operation - * will detect this during retries and fail. + * If a concurrent operation commits a new data or delete file after the table was queried and + * that file might contain either new records or relevant deletes matching the specified conflict + * detection filter, the overwrite operation will detect this during retries and fail. * <p> * Calling this method with a correct conflict detection filter is required to maintain * serializable isolation for eager update/delete operations. Otherwise, the isolation level * will be snapshot isolation. + * <p> + * Validation applies to operations happened since the snapshot passed to {@link #validateFromSnapshot(long)}. * - * @param readSnapshotId the snapshot id that was used to read the data or null if the table was empty * @param conflictDetectionFilter an expression on rows in the table * @return this for method chaining - * @deprecated this will be removed in 0.11.0; - * use {@link #validateNoConflictingAppends(Expression)} and {@link #validateFromSnapshot(long)} instead */ - @Deprecated - OverwriteFiles validateNoConflictingAppends(Long readSnapshotId, Expression conflictDetectionFilter); + OverwriteFiles validateNoConflictingOperations(Expression conflictDetectionFilter); Review comment: Looks like this method is designed to validate whether the copy-on-write batch overwrite operations will be conflicted with the RowDelta operations ? I will need some time to read the whole copy-on-write code path. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org