viongpanzi commented on a change in pull request #7716: AggregatorUtil should
cache parsed expression to avoid memory problem (OOM/FGC) when Expression is
used in metricsSpec
URL: https://github.com/apache/incubator-druid/pull/7716#discussion_r286778106
##########
File path:
processing/src/main/java/org/apache/druid/query/aggregation/AggregatorUtil.java
##########
@@ -196,7 +246,7 @@ static BaseFloatColumnValueSelector
makeColumnValueSelectorWithFloatDefault(
if (fieldName != null) {
return metricFactory.makeColumnValueSelector(fieldName);
} else {
- final Expr expr = Parser.parse(fieldExpression, macroTable);
+ final Expr expr = parseIfAbsent(fieldExpression, macroTable);
Review comment:
@himanshug Would it be better if we move the cache to the Parser class and
add a new method like parseIfAbsent? Otherwise, we need to add cache in
SimpleXXAggregatorFactory, respectively.
```
public abstract class SimpleFloatAggregatorFactory extends
NullableAggregatorFactory<BaseFloatColumnValueSelector>
{
protected final String name;
@Nullable
protected final String fieldName;
@Nullable
protected final String expression;
protected final ExprMacroTable macroTable;
protected final Supplier<Expr> fieldExpression;
public SimpleFloatAggregatorFactory(
ExprMacroTable macroTable,
String name,
@Nullable final String fieldName,
@Nullable String expression
)
{
this.macroTable = macroTable;
this.name = name;
this.fieldName = fieldName;
this.expression = expression;
this.fieldExpression = () -> expression == null ? null :
Parser.parseIfAbsent(expression, macroTable);
```
----------------------------------------------------------------
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]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
