rdblue commented on a change in pull request #968:
URL: https://github.com/apache/incubator-iceberg/pull/968#discussion_r415405306



##########
File path: core/src/main/java/org/apache/iceberg/SnapshotManager.java
##########
@@ -50,25 +51,34 @@ protected ManageSnapshots self() {
 
   @Override
   protected String operation() {
-    // snapshotOperation is used by SnapshotProducer when building and writing 
a new snapshot for cherrypick
-    Preconditions.checkNotNull(snapshotOperation, "[BUG] Detected 
uninitialized operation");
+    //TODO: We are not setting this var during rollbacks. What should its 
value be during rollbacks?
     return snapshotOperation;
   }
 
+  @Override
+  protected long snapshotId() {
+    if (requiresSnapshotCreation) {
+      return super.snapshotId();
+    }
+    // No new snapshot was created
+    return targetSnapshotId;
+  }
+
   @Override
   public ManageSnapshots cherrypick(long snapshotId) {
     TableMetadata current = current();
     ValidationException.check(current.snapshot(snapshotId) != null,
         "Cannot cherry pick unknown snapshot id: %s", snapshotId);
 
     Snapshot cherryPickSnapshot = current.snapshot(snapshotId);
+    // TODO: shouldn't we check for appends only if cherrypick op is not fast 
forward-able?
     // only append operations are currently supported
     if (cherryPickSnapshot.operation().equals(DataOperations.APPEND)) {
       this.managerOperation = SnapshotManagerOperation.CHERRYPICK;
       this.targetSnapshotId = snapshotId;
       this.snapshotOperation = cherryPickSnapshot.operation();
-
       // Pick modifications from the snapshot
+      //TODO: seems like this is wasted work if the cherrypick op is fast 
forward-able and only has appends operations

Review comment:
       The case is what I described above: if the fast-forward fails due to a 
conflict, it will fall back to cherry-pick.




----------------------------------------------------------------
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.

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

Reply via email to