RussellSpitzer commented on code in PR #12593:
URL: https://github.com/apache/iceberg/pull/12593#discussion_r2012487663


##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -283,32 +284,55 @@ public Snapshot apply() {
       throw new RuntimeIOException(e, "Failed to write manifest list file");
     }
 
+    Map<String, String> summary = summary();
+    String operation = operation();
+
     Long addedRows = null;
-    Long lastRowId = null;
-    if (base.rowLineageEnabled()) {
-      addedRows = calculateAddedRows(manifests);
-      lastRowId = base.nextRowId();
+    Long firstRowId = null;
+    if (base.formatVersion() >= 3) {
+      addedRows = calculateAddedRows(operation, summary, manifests);
+      firstRowId = base.nextRowId();
     }
 
     return new BaseSnapshot(
         sequenceNumber,
         snapshotId(),
         parentSnapshotId,
         System.currentTimeMillis(),
-        operation(),
-        summary(base),
+        operation,
+        summaryWithTotals(base, summary),
         base.currentSchemaId(),
         manifestList.location(),
-        lastRowId,
+        firstRowId,
         addedRows);
   }
 
-  private Long calculateAddedRows(List<ManifestFile> manifests) {
+  private Long calculateAddedRows(
+      String operation, Map<String, String> summary, List<ManifestFile> 
manifests) {
+    if (summary != null) {
+      long addedRecords =
+          PropertyUtil.propertyAsLong(summary, 
SnapshotSummary.ADDED_RECORDS_PROP, 0L);
+      if (DataOperations.REPLACE.equals(operation)) {
+        long replacedRecords =
+            PropertyUtil.propertyAsLong(summary, 
SnapshotSummary.DELETED_RECORDS_PROP, 0L);
+        // added may be less than replaced when records are already deleted by 
delete files
+        Preconditions.checkArgument(

Review Comment:
   I got worried about this again now, just because the property is there for 
DELETE_RECORDS or not, doesn't mean we can assume addedRecords is correct. I 
think we are biasing a bit too much towards the Java impl here.
   



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

Reply via email to