rdblue commented on code in PR #18108:
URL: https://github.com/apache/iceberg/pull/18108#discussion_r4138439381


##########
core/src/main/java/org/apache/iceberg/TrackedFileStruct.java:
##########
@@ -210,14 +214,34 @@ public long fileSizeInBytes() {
     return fileSizeInBytes;
   }
 
+  void setPartitionType(Types.StructType newPartitionType) {
+    this.projectedPartition = project(partitionData, newPartitionType);
+  }
+
   @Override
   public Integer specId() {
     return specId;
   }
 
   @Override
   public StructLike partition() {
-    return partitionData;
+    return projectedPartition != null ? projectedPartition : partitionData;
+  }
+
+  private static PartitionData copyOf(PartitionData partition) {
+    return partition != null ? partition.copy() : null;
+  }
+
+  private static PartitionData project(PartitionData partition, 
Types.StructType partitionType) {

Review Comment:
   I don't think that this logic should be done for every row. There are fairly 
expensive operations here (comparing the partition tuple and building a 
projection) that don't need to be done in a tight loop because the result can 
be cached based on the spec ID.
   
   It looks like part of the motivation here is to continue using 
`partition.getPartitionType()`, but that is only necessary if the requested 
projection is unknown. It will be cleaner to change the underlying partition to 
a `StructLike` and avoid using its partition-specific methods.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to