gortiz commented on code in PR #8766:
URL: https://github.com/apache/pinot/pull/8766#discussion_r884466651


##########
pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ColumnValueSegmentPruner.java:
##########
@@ -435,4 +469,74 @@ private static Comparable convertValue(String stringValue, 
DataType dataType) {
       throw new BadQueryRequestException(e);
     }
   }
+
+  private static class ValueCache {
+    // As Predicates are recursive structures, their hashCode is quite 
expensive.
+    // By using an IdentityHashMap here we don't need to iterate over the 
recursive
+    // structure. This is specially useful in the IN expression.
+    private final Map<Predicate, Object> _cache = new IdentityHashMap<>();
+
+    public void add(EqPredicate pred) {
+      _cache.put(pred, new CachedValue(pred.getValue()));
+    }
+
+    public void add(InPredicate pred) {
+      List<CachedValue> list = new ArrayList<>(pred.getValues().size());
+      for (String value : pred.getValues()) {
+        list.add(new CachedValue(value));
+      }
+      _cache.put(pred, list);
+    }
+
+    public CachedValue get(EqPredicate pred, DataType dt) {
+      CachedValue cachedValue = (CachedValue) _cache.get(pred);
+      cachedValue.ensureDataType(dt);
+      return cachedValue;
+    }
+
+    public List<CachedValue> get(InPredicate pred, DataType dt) {
+      List<CachedValue> cachedValues = (List<CachedValue>) _cache.get(pred);
+      for (CachedValue cachedValue : cachedValues) {
+        cachedValue.ensureDataType(dt);
+      }
+      return cachedValues;
+    }
+
+    public static class CachedValue {
+      private final Object _value;
+      private boolean _hashed = false;
+      private long _hash1;
+      private long _hash2;
+      private DataType _dt;
+      private Comparable _comparableValue;
+
+      private CachedValue(Object value) {
+        _value = value;
+      }
+
+      private Comparable getComparableValue() {
+        assert _dt != null;
+        return _comparableValue;
+      }
+
+      private void ensureDataType(DataType dt) {
+        if (!dt.equals(_dt)) {

Review Comment:
   Are you used to compare enums by reference? In my experience, it is quite 
dangerous. It should happen that there is only one instance for each literal... 
but sometimes deserialize some enum using reflection and creates more than one 
instance and the equality by reference start to fail.



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