amogh-jahagirdar commented on code in PR #11948:
URL: https://github.com/apache/iceberg/pull/11948#discussion_r1928151867


##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -290,7 +298,27 @@ public Snapshot apply() {
         operation(),
         summary(base),
         base.currentSchemaId(),
-        manifestList.location());
+        manifestList.location(),
+        lastRowId,
+        addedRows);
+  }
+
+  private Long calculateAddedRows(List<ManifestFile> manifests) {
+    return manifests.stream()
+        .filter(
+            manifest ->
+                manifest.snapshotId() == null
+                    || Objects.equals(manifest.snapshotId(), this.snapshotId))
+        .mapToLong(
+            manifest -> {
+              Preconditions.checkArgument(
+                  manifest.addedRowsCount() != null,
+                  "Cannot determine number of added rows in snapshot because"
+                      + " the entry for manifest %s is missing the field 
`added-rows-count`",
+                  manifest.path());

Review Comment:
   Ah I checked this out locally and wrote a test...addedRowsCount would just 
be 0 in that case. Cool 



##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -290,7 +298,27 @@ public Snapshot apply() {
         operation(),
         summary(base),
         base.currentSchemaId(),
-        manifestList.location());
+        manifestList.location(),
+        lastRowId,
+        addedRows);
+  }
+
+  private Long calculateAddedRows(List<ManifestFile> manifests) {
+    return manifests.stream()
+        .filter(
+            manifest ->
+                manifest.snapshotId() == null
+                    || Objects.equals(manifest.snapshotId(), this.snapshotId))
+        .mapToLong(
+            manifest -> {
+              Preconditions.checkArgument(
+                  manifest.addedRowsCount() != null,
+                  "Cannot determine number of added rows in snapshot because"
+                      + " the entry for manifest %s is missing the field 
`added-rows-count`",
+                  manifest.path());

Review Comment:
   Ah I checked this out locally and wrote a test...I confused myself 
originally, addedRowsCount would just be 0 in that case not null. That makes 
sense!  



##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -290,7 +298,27 @@ public Snapshot apply() {
         operation(),
         summary(base),
         base.currentSchemaId(),
-        manifestList.location());
+        manifestList.location(),
+        lastRowId,
+        addedRows);
+  }
+
+  private Long calculateAddedRows(List<ManifestFile> manifests) {
+    return manifests.stream()
+        .filter(
+            manifest ->
+                manifest.snapshotId() == null
+                    || Objects.equals(manifest.snapshotId(), this.snapshotId))
+        .mapToLong(
+            manifest -> {
+              Preconditions.checkArgument(
+                  manifest.addedRowsCount() != null,
+                  "Cannot determine number of added rows in snapshot because"
+                      + " the entry for manifest %s is missing the field 
`added-rows-count`",
+                  manifest.path());

Review Comment:
   Ah I checked this out locally and wrote a test...I confused myself 
originally, addedRowsCount would just be 0 in that case not null. That makes 
sense!  It's also how the general delete case works (which has a test)



##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -290,7 +298,27 @@ public Snapshot apply() {
         operation(),
         summary(base),
         base.currentSchemaId(),
-        manifestList.location());
+        manifestList.location(),
+        lastRowId,
+        addedRows);
+  }
+
+  private Long calculateAddedRows(List<ManifestFile> manifests) {
+    return manifests.stream()
+        .filter(
+            manifest ->
+                manifest.snapshotId() == null
+                    || Objects.equals(manifest.snapshotId(), this.snapshotId))
+        .mapToLong(
+            manifest -> {
+              Preconditions.checkArgument(
+                  manifest.addedRowsCount() != null,
+                  "Cannot determine number of added rows in snapshot because"
+                      + " the entry for manifest %s is missing the field 
`added-rows-count`",
+                  manifest.path());

Review Comment:
   Ah I checked this out locally and wrote a test...I confused myself 
originally, addedRowsCount would just be 0 in that case not null. That makes 
sense!  It's also how the general delete file case works (which has a test)



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