raunaqmorarka commented on code in PR #5908:
URL: https://github.com/apache/iceberg/pull/5908#discussion_r1930020478


##########
api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java:
##########
@@ -258,6 +270,23 @@ public <T> String predicate(UnboundPredicate<T> pred) {
     }
   }
 
+  private static <T> List<String> abbreviateValues(List<String> 
sanitizedValues) {
+    if (sanitizedValues.size() >= LONG_IN_PREDICATE_ABBREVIATION_THRESHOLD) {
+      Set<String> distinctValues = ImmutableSet.copyOf(sanitizedValues);
+      if (distinctValues.size()
+          <= sanitizedValues.size() - LONG_IN_PREDICATE_ABBREVIATION_MIN_GAIN) 
{

Review Comment:
   This logic of de-duplicating sanitized values works well only when we're 
dealing with integers because sanitized integers are represented by a limited 
set of values like `2-digit-int`, `3-digit-int` etc.
   But when we have string values which are sanitized to hashes (e.g. 
`(hash-1b409883), (hash-53cd6d46), (hash-24add70a), (hash-7df3cf93), ...` this 
logic isn't helpful in abbrevaiting long IN list.
   Since we're already dealing with sanitized values, I'm wondering why we're 
making the effort to always print all the distinct sanitized values ? 
   Can we make this simpler and just print first 2 and last distinct sanitized 
values when the number of distinctValues exceeds some threshold ?



-- 
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: issues-unsubscr...@iceberg.apache.org

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

Reply via email to