clintropolis commented on a change in pull request #11358:
URL: https://github.com/apache/druid/pull/11358#discussion_r649685362
##########
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:
I was worried about that too, but hesitated since `Expr` is in core and
this is in processing and also some others such as the `DimensionSpec` have
this problem, but yeah I agree and fixed it and moved both of them to `Exprs`
because wasn't sure where else to put it and it seemed sort of funny on `Expr`.
##########
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:
added caching similar to the `Expr` themselves
##########
File path: core/src/main/java/org/apache/druid/math/expr/Exprs.java
##########
@@ -29,6 +29,9 @@
public class Exprs
{
+ public static final byte EXPR_CACHE_KEY = 0x00;
+ public static final byte LOOKUP_EXPR_KEY = 0x01;
Review comment:
oops, meant to rename to `LOOKUP_EXPR_CACHE_KEY` when i moved here, fixed
--
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]
