stevenzwu commented on code in PR #18394:
URL: https://github.com/apache/iceberg/pull/18394#discussion_r4192226353


##########
core/src/main/java/org/apache/iceberg/FieldStatsStruct.java:
##########
@@ -231,6 +232,9 @@ private static int[] posToOffset(Types.StructType struct) {
   private static Object copyBound(Object bound) {
     if (bound instanceof byte[] bytes) {
       return copyOf(bytes);
+    } else if (bound instanceof GeospatialBound geo) {
+      // readers reuse one bound instance
+      return geo.copy();
     } else if (bound instanceof StructLike struct) {
       return StructLikeUtil.copy(struct);

Review Comment:
   We kept the generic StructLike copy because 
TestFieldStatsStruct.geoSerialization still builds lower and upper bounds as 
PartitionData. FieldStats#copy needs StructLikeUtil.copy so the copy is not the 
same instance. Dropping this branch fails the geometry and geography 
FieldStats#copy cases on isNotSameAs. Other serializers in that test still pass 
because the round trip allocates a new object.
   
   We are exploring removing this branch and adjusting the unit tests to use 
GeospatialBound instead of PartitionData.



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