Re: [PR] fix when equalityFieldColumns is not null and upsert is false, position delete in write function will lead to unstable result if flink checkpoint interval is not same [iceberg]

[via GitHub]

sunnyzhuzhu commented on code in PR #9300:
URL: https://github.com/apache/iceberg/pull/9300#discussion_r1427494930


##########
core/src/main/java/org/apache/iceberg/io/BaseTaskWriter.java:
##########
@@ -137,11 +137,7 @@ public void write(T row) throws IOException {
       StructLike copiedKey = 
StructCopy.copy(structProjection.wrap(asStructLike(row)));
 
       // Adding a pos-delete to replace the old path-offset.
-      PathOffset previous = insertedRowMap.put(copiedKey, pathOffset);
-      if (previous != null) {
-        // TODO attach the previous row if has a positional-delete row schema 
in appender factory.
-        posDeleteWriter.delete(previous.path, previous.rowOffset, null);
-      }

Review Comment:
   > Removing this doesn't seem like a good idea. Shouldn't we have to write a 
position delete row if a previously inserted row is updated?
   
   Thanks for your review!
   previous is always null when upsert is true, as 
BaseEqualityDeltaWriter.deleteKey() is called when upsert is true. Otherwise, 
when write.upsert.enabled is false, I think it should not delete previous row.  
   Here I modify the code for compatibility, can you help to review again, 
thank you very much!



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