metanil commented on code in PR #14277:
URL: https://github.com/apache/iceberg/pull/14277#discussion_r2414518457


##########
api/src/main/java/org/apache/iceberg/types/Comparators.java:
##########
@@ -76,6 +76,11 @@ public static <T> Comparator<T> forType(Type.PrimitiveType 
type) {
     throw new UnsupportedOperationException("Cannot determine comparator for 
type: " + type);
   }
 
+  @SuppressWarnings("unchecked")
+  public static <T> Comparator<T> forType(Type type) {
+    return internal(type);
+  }
+
   @SuppressWarnings("unchecked")
   private static <T> Comparator<T> internal(Type type) {

Review Comment:
   Since the new method above is just making this method from `private` to 
`public`, there are couple of options to avoid it (that I can think of).
   Option 1: Just rename this and make it `public`.
   Option 2: No change in `Comparators.java`, Make partition ordering **only** 
on primitive types. Need to do `type.isPrimitiveType()` check down in 
`SparkPartitionAwareScan.java`



##########
api/src/main/java/org/apache/iceberg/types/Comparators.java:
##########
@@ -76,6 +76,11 @@ public static <T> Comparator<T> forType(Type.PrimitiveType 
type) {
     throw new UnsupportedOperationException("Cannot determine comparator for 
type: " + type);
   }
 
+  @SuppressWarnings("unchecked")
+  public static <T> Comparator<T> forType(Type type) {
+    return internal(type);
+  }
+
   @SuppressWarnings("unchecked")
   private static <T> Comparator<T> internal(Type type) {

Review Comment:
   Since the new method above is just making this method from `private` to 
`public`, there are couple of options to avoid it (that I can think of).
   1. Option 1: Just rename this and make it `public`.
   2. Option 2: No change in `Comparators.java`, Make partition ordering 
**only** on primitive types. Need to do `type.isPrimitiveType()` check down in 
`SparkPartitionAwareScan.java`



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