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