[GitHub] [druid] gianm commented on a change in pull request #11358: enrich expression cache key information to support expressions which depend on external state

[GitBox <gi...@apache.org>]

gianm commented on a change in pull request #11358:
URL: https://github.com/apache/druid/pull/11358#discussion_r649621811



##########
File path: core/src/main/java/org/apache/druid/math/expr/Expr.java
##########
@@ -171,6 +174,12 @@ default boolean canVectorize(InputBindingInspector inspector)
     throw Exprs.cannotVectorize(this);
   }
 
+  @Override
+  default byte[] getCacheKey()
+  {
+    return new CacheKeyBuilder(EXPR_CACHE_KEY).appendString(stringify()).build();

Review comment:
       `stringify` is somewhat expensive and this will be called once per segment. Can we cache it, like we cache the original and parsed expressions? Best way I can think of right now is to implement that caching in ExpressionFilter, ExpressionVirtualColumn, etc.
   
   Btw, that caching should be lazy, for two reasons:
   
   - otherwise we'll waste time computing it when it'll never be needed
   - sometimes the cache key will be incomputable (lookup doesn't exist, or error retrieving it) and we don't want that to totally prevent instantiating the VirtualColumn or DimFilter objects

##########
File path: processing/src/main/java/org/apache/druid/query/expression/LookupExprMacro.java
##########
@@ -36,6 +37,7 @@
 
 public class LookupExprMacro implements ExprMacroTable.ExprMacro
 {
+  private static final byte LOOKUP_EXPR_KEY = 0x01;

Review comment:
       Can we centralize these? Otherwise I worry about accidentally reusing IDs.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org