[GitHub] [incubator-iceberg] rdblue commented on a change in pull request #875: Spark: Implement an action to rewrite manifests

[GitBox]

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



##########
File path: spark/src/main/java/org/apache/iceberg/spark/SparkDataFile.java
##########
@@ -74,42 +105,44 @@ public StructLike partition() {
 
   @Override
   public long recordCount() {
-    return wrapped.getAs(fieldPositions[3]);
+    return wrapped.getAs(recordCountPosition);
   }
 
   @Override
   public long fileSizeInBytes() {
-    return wrapped.getAs(fieldPositions[4]);
+    return wrapped.getAs(fileSizeInBytesPosition);
   }
 
   @Override
   public Map<Integer, Long> columnSizes() {
-    return wrapped.getJavaMap(fieldPositions[6]);
+    return !wrapped.isNullAt(columnSizesPosition) ? 
wrapped.getJavaMap(columnSizesPosition) : null;
   }
 
   @Override
   public Map<Integer, Long> valueCounts() {
-    return wrapped.getJavaMap(fieldPositions[7]);
+    return !wrapped.isNullAt(valueCountsPosition) ? 
wrapped.getJavaMap(valueCountsPosition) : null;
   }
 
   @Override
   public Map<Integer, Long> nullValueCounts() {
-    return wrapped.getJavaMap(fieldPositions[8]);
+    return !wrapped.isNullAt(nullValueCountsPosition) ? 
wrapped.getJavaMap(nullValueCountsPosition) : null;

Review comment:
       Nit: I generally prefer reversing the order instead of using `!`:
   * It's usually easier to understand the positive: `if isNullAt ? null : ...`
   * Changing these later with a second condition is easier if the condition is 
positive




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