huan233usc commented on code in PR #16850:
URL: https://github.com/apache/iceberg/pull/16850#discussion_r3433155356


##########
parquet/src/main/java/org/apache/iceberg/parquet/ParquetMetrics.java:
##########
@@ -264,13 +266,19 @@ private FieldMetrics<ByteBuffer> metricsFromFooter(
         int truncateLength) {
       if (primitive.getPrimitiveTypeName() == 
PrimitiveType.PrimitiveTypeName.INT96) {
         return null;
-      } else if (truncateLength <= 0) {
+      } else if (truncateLength <= 0 || isGeospatial(icebergType)) {
+        // Parquet lexicographic min/max is not meaningful for spatial WKB.
         return counts(fieldId);
       } else {
         return bounds(fieldId, icebergType, primitive, truncateLength);
       }
     }
 
+    private static boolean 
isGeospatial(org.apache.iceberg.types.Type.PrimitiveType icebergType) {
+      TypeID typeId = Preconditions.checkNotNull(icebergType, "Invalid type: 
null").typeId();

Review Comment:
   Thanks, fixed in `de6703b60`. The call site now preserves the existing null 
fallback with `icebergType != null && ...`, so null still flows to 
`bounds(...)` and returns null as before.



##########
parquet/src/main/java/org/apache/iceberg/parquet/ParquetMetrics.java:
##########
@@ -264,13 +266,19 @@ private FieldMetrics<ByteBuffer> metricsFromFooter(
         int truncateLength) {
       if (primitive.getPrimitiveTypeName() == 
PrimitiveType.PrimitiveTypeName.INT96) {
         return null;
-      } else if (truncateLength <= 0) {
+      } else if (truncateLength <= 0 || isGeospatial(icebergType)) {
+        // Parquet lexicographic min/max is not meaningful for spatial WKB.
         return counts(fieldId);
       } else {
         return bounds(fieldId, icebergType, primitive, truncateLength);
       }
     }
 
+    private static boolean 
isGeospatial(org.apache.iceberg.types.Type.PrimitiveType icebergType) {

Review Comment:
   Also applied this suggestion in `de6703b60`: `isGeospatial` now takes a 
`TypeID`, so the helper no longer needs the fully-qualified Iceberg 
`PrimitiveType` parameter.



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