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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
