singhpk234 commented on code in PR #5888:
URL: https://github.com/apache/iceberg/pull/5888#discussion_r1120865320
##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -615,4 +630,49 @@ private static void updateTotal(
}
}
}
+
+ private Long snapshotIdToRollbackToOnConflict(Long parentSnapshotId) {
+ Long newParentSnapshotId = parentSnapshotId;
+ if (shouldRollbackReplaceOnConflict()) {
+ // add a set snapshot op on top of base to roll back to parent snapshot
+ boolean isCommitSuccessfullyApplied = false;
+ // Update parentSnapshot to it's grandParent
Review Comment:
yup
##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -215,7 +219,18 @@ public Snapshot apply() {
long sequenceNumber = base.nextSequenceNumber();
Long parentSnapshotId = parentSnapshot == null ? null :
parentSnapshot.snapshotId();
- validate(base, parentSnapshot);
+ try {
+ validate(base, parentSnapshot);
+ } catch (ValidationException ve) {
+ parentSnapshotId = snapshotIdToRollbackToOnConflict(parentSnapshotId);
Review Comment:
sounds good, made the changes
##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -615,4 +630,49 @@ private static void updateTotal(
}
}
}
+
+ private Long snapshotIdToRollbackToOnConflict(Long parentSnapshotId) {
+ Long newParentSnapshotId = parentSnapshotId;
+ if (shouldRollbackReplaceOnConflict()) {
+ // add a set snapshot op on top of base to roll back to parent snapshot
+ boolean isCommitSuccessfullyApplied = false;
+ // Update parentSnapshot to it's grandParent
+ // provided parentSnapshot is of type replace and never rollback beyond
startingSnapshotId
+ while (newParentSnapshotId != null
+ &&
DataOperations.REPLACE.equals(base.snapshot(newParentSnapshotId).operation())
+ && !newParentSnapshotId.equals(startingSnapshotId())) {
+ // create a tempTableOperation to pass base with rollback to validate
of update
+ TableOperations tempTableOps = ops.temp(base);
+ newParentSnapshotId = base.snapshot(newParentSnapshotId).parentId();
+ if (newParentSnapshotId == null) {
+ return null;
+ }
+ Snapshot parentSnapshot = base.snapshot(newParentSnapshotId);
+
+ SetSnapshotOperation setSnapshotOp = new
SetSnapshotOperation(tempTableOps);
+ setSnapshotOp.rollbackTo(newParentSnapshotId).commit();
Review Comment:
As per my understanding, since this is based on `TableOperations
tempTableOps = ops.temp(base);` so it should not commit, we use this for
validation to create a new temp metadata which will act as an uncommitted table
metadata similar to what's done in transaction.
--
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
