Skip to content

Commit e239113

Browse files
julian-elasticjulian-elastic
authored
Remove unnecessary calls to fold() (#131870)
This PR removes additional calls to Fold from various function code. With this change all of the functions are handled except DateTrunc. This is part 2 of 3 to remove folding. There are additional places marked with /* TODO remove me */ in the Analyzer and LogicalPlanBuilder and will be handled in a separate PR.
1 parent d39aeec commit e239113

File tree

34 files changed

+424
-144
lines changed

34 files changed

+424
-144
lines changed

‎x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Foldables.java‎

Lines changed: 0 additions & 27 deletions
This file was deleted.

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/EnrichPolicyResolver.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@
5959
import java.util.Set;
6060
import java.util.stream.Collectors;
6161

62-
import static org.elasticsearch.xpack.esql.core.expression.Foldables.stringLiteralValueOf;
62+
import static org.elasticsearch.xpack.esql.expression.Foldables.stringLiteralValueOf;
6363
import static org.elasticsearch.xpack.esql.session.EsqlCCSUtils.markClusterWithFinalStateAndNoShards;
6464

6565
/**

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/FunctionUtils.java‎ renamed to ‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/Foldables.java‎

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,24 @@
44
* 2.0; you may not use this file except in compliance with the Elastic License
55
* 2.0.
66
*/
7-
package org.elasticsearch.xpack.esql.expression.function;
7+
package org.elasticsearch.xpack.esql.expression;
88

9+
import org.apache.lucene.util.BytesRef;
910
import org.elasticsearch.common.lucene.BytesRefs;
1011
import org.elasticsearch.core.Nullable;
12+
import org.elasticsearch.core.Strings;
1113
import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
1214
import org.elasticsearch.xpack.esql.common.Failure;
1315
import org.elasticsearch.xpack.esql.common.Failures;
16+
import org.elasticsearch.xpack.esql.core.QlIllegalArgumentException;
1417
import org.elasticsearch.xpack.esql.core.expression.Expression;
18+
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
1519
import org.elasticsearch.xpack.esql.core.expression.Literal;
1620

1721
import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
1822
import static org.elasticsearch.xpack.esql.common.Failure.fail;
1923

20-
public class FunctionUtils {
24+
public abstract class Foldables {
2125
/**
2226
* A utility class to validate the type resolution of expressions before and after logical planning.
2327
* If null is passed for Failures to the constructor, it means we are only type resolution.
@@ -64,6 +68,34 @@ public Expression.TypeResolution getResolvedType() {
6468
}
6569
}
6670

71+
public static Object valueOf(FoldContext ctx, Expression e) {
72+
if (e.foldable()) {
73+
return e.fold(ctx);
74+
}
75+
throw new QlIllegalArgumentException("Cannot determine value for {}", e);
76+
}
77+
78+
public static String stringLiteralValueOf(Expression expression, String message) {
79+
if (expression instanceof Literal literal && literal.value() instanceof BytesRef bytesRef) {
80+
return bytesRef.utf8ToString();
81+
}
82+
throw new QlIllegalArgumentException(message);
83+
}
84+
85+
public static Object literalValueOf(Expression e) {
86+
if (e instanceof Literal literal) {
87+
return literal.value();
88+
}
89+
throw new QlIllegalArgumentException("Expected literal, but got {}", e);
90+
}
91+
92+
public static Object extractLiteralOrReturnSelf(Expression e) {
93+
if (e instanceof Literal literal) {
94+
return literal.value();
95+
}
96+
return e;
97+
}
98+
6799
public static Integer limitValue(Expression limitField, String sourceText) {
68100
if (limitField instanceof Literal literal) {
69101
Object value = literal.value();
@@ -148,4 +180,13 @@ public static String queryAsString(Expression queryField, String sourceText) {
148180
format(null, "Query value must be a constant string in [{}], found [{}]", sourceText, queryField)
149181
);
150182
}
183+
184+
public static int intValueOf(Expression field, String sourceText, String fieldName) {
185+
if (field instanceof Literal literal && literal.value() instanceof Number n) {
186+
return n.intValue();
187+
}
188+
throw new EsqlIllegalArgumentException(
189+
Strings.format(null, "[{}] value must be a constant number in [{}], found [{}]", fieldName, sourceText, field)
190+
);
191+
}
151192
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/AggregateFunction.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import org.elasticsearch.xpack.esql.capabilities.PostAnalysisPlanVerificationAware;
1313
import org.elasticsearch.xpack.esql.common.Failures;
1414
import org.elasticsearch.xpack.esql.core.expression.Expression;
15-
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
1615
import org.elasticsearch.xpack.esql.core.expression.Literal;
1716
import org.elasticsearch.xpack.esql.core.expression.TypeResolutions;
1817
import org.elasticsearch.xpack.esql.core.expression.function.Function;
@@ -130,7 +129,7 @@ public List<? extends Expression> parameters() {
130129

131130
public boolean hasFilter() {
132131
return filter != null
133-
&& (filter.foldable() == false || Boolean.TRUE.equals(filter.fold(FoldContext.small() /* TODO remove me */)) == false);
132+
&& (filter.foldable() == false || (filter instanceof Literal literal && Boolean.TRUE.equals(literal.value()) == false));
134133
}
135134

136135
public Expression filter() {

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/CountDistinct.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import org.elasticsearch.compute.aggregation.CountDistinctLongAggregatorFunctionSupplier;
2020
import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
2121
import org.elasticsearch.xpack.esql.core.expression.Expression;
22-
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
2322
import org.elasticsearch.xpack.esql.core.expression.Literal;
2423
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
2524
import org.elasticsearch.xpack.esql.core.tree.Source;
@@ -49,6 +48,7 @@
4948
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isType;
5049
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isWholeNumber;
5150
import static org.elasticsearch.xpack.esql.core.util.CollectionUtils.nullSafeList;
51+
import static org.elasticsearch.xpack.esql.expression.Foldables.intValueOf;
5252

5353
public class CountDistinct extends AggregateFunction implements OptionalArgument, ToAggregator, SurrogateExpression {
5454
public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(
@@ -210,16 +210,18 @@ protected TypeResolution resolveType() {
210210
@Override
211211
public AggregatorFunctionSupplier supplier() {
212212
DataType type = field().dataType();
213-
int precision = this.precision == null
214-
? DEFAULT_PRECISION
215-
: ((Number) this.precision.fold(FoldContext.small() /* TODO remove me */)).intValue();
213+
int precision = this.precision == null ? DEFAULT_PRECISION : precisionValue();
216214
if (SUPPLIERS.containsKey(type) == false) {
217215
// If the type checking did its job, this should never happen
218216
throw EsqlIllegalArgumentException.illegalDataType(type);
219217
}
220218
return SUPPLIERS.get(type).apply(precision);
221219
}
222220

221+
private int precisionValue() {
222+
return intValueOf(precision, source().text(), "Precision");
223+
}
224+
223225
@Override
224226
public Expression surrogate() {
225227
var s = source();

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Percentile.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import org.elasticsearch.compute.aggregation.PercentileIntAggregatorFunctionSupplier;
1717
import org.elasticsearch.compute.aggregation.PercentileLongAggregatorFunctionSupplier;
1818
import org.elasticsearch.xpack.esql.core.expression.Expression;
19-
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
2019
import org.elasticsearch.xpack.esql.core.expression.Literal;
2120
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
2221
import org.elasticsearch.xpack.esql.core.tree.Source;
@@ -38,6 +37,7 @@
3837
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.SECOND;
3938
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isFoldable;
4039
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isType;
40+
import static org.elasticsearch.xpack.esql.expression.Foldables.intValueOf;
4141

4242
public class Percentile extends NumericAggregate implements SurrogateExpression {
4343
public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(
@@ -169,7 +169,7 @@ protected AggregatorFunctionSupplier doubleSupplier() {
169169
}
170170

171171
private int percentileValue() {
172-
return ((Number) percentile.fold(FoldContext.small() /* TODO remove me */)).intValue();
172+
return intValueOf(percentile(), source().text(), "Percentile");
173173
}
174174

175175
@Override

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Sample.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@
2424
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
2525
import org.elasticsearch.xpack.esql.core.tree.Source;
2626
import org.elasticsearch.xpack.esql.core.type.DataType;
27+
import org.elasticsearch.xpack.esql.expression.Foldables;
2728
import org.elasticsearch.xpack.esql.expression.function.Example;
2829
import org.elasticsearch.xpack.esql.expression.function.FunctionAppliesTo;
2930
import org.elasticsearch.xpack.esql.expression.function.FunctionAppliesToLifecycle;
3031
import org.elasticsearch.xpack.esql.expression.function.FunctionInfo;
3132
import org.elasticsearch.xpack.esql.expression.function.FunctionType;
32-
import org.elasticsearch.xpack.esql.expression.function.FunctionUtils;
3333
import org.elasticsearch.xpack.esql.expression.function.Param;
3434
import org.elasticsearch.xpack.esql.planner.PlannerUtils;
3535
import org.elasticsearch.xpack.esql.planner.ToAggregator;
@@ -42,9 +42,9 @@
4242
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isNotNull;
4343
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isRepresentableExceptCounters;
4444
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isType;
45-
import static org.elasticsearch.xpack.esql.expression.function.FunctionUtils.TypeResolutionValidator.forPostOptimizationValidation;
46-
import static org.elasticsearch.xpack.esql.expression.function.FunctionUtils.TypeResolutionValidator.forPreOptimizationValidation;
47-
import static org.elasticsearch.xpack.esql.expression.function.FunctionUtils.resolveTypeLimit;
45+
import static org.elasticsearch.xpack.esql.expression.Foldables.TypeResolutionValidator.forPostOptimizationValidation;
46+
import static org.elasticsearch.xpack.esql.expression.Foldables.TypeResolutionValidator.forPreOptimizationValidation;
47+
import static org.elasticsearch.xpack.esql.expression.Foldables.resolveTypeLimit;
4848

4949
public class Sample extends AggregateFunction implements ToAggregator, PostOptimizationVerificationAware {
5050
public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(Expression.class, "Sample", Sample::new);
@@ -174,7 +174,7 @@ Expression limitField() {
174174
}
175175

176176
private int limitValue() {
177-
return FunctionUtils.limitValue(limitField(), sourceText());
177+
return Foldables.limitValue(limitField(), sourceText());
178178
}
179179

180180
Expression uuid() {
@@ -183,6 +183,6 @@ Expression uuid() {
183183

184184
@Override
185185
public void postOptimizationVerification(Failures failures) {
186-
FunctionUtils.resolveTypeLimit(limitField(), sourceText(), forPostOptimizationValidation(limitField(), failures));
186+
Foldables.resolveTypeLimit(limitField(), sourceText(), forPostOptimizationValidation(limitField(), failures));
187187
}
188188
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Top.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@
2727
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
2828
import org.elasticsearch.xpack.esql.core.tree.Source;
2929
import org.elasticsearch.xpack.esql.core.type.DataType;
30+
import org.elasticsearch.xpack.esql.expression.Foldables;
31+
import org.elasticsearch.xpack.esql.expression.Foldables.TypeResolutionValidator;
3032
import org.elasticsearch.xpack.esql.expression.SurrogateExpression;
3133
import org.elasticsearch.xpack.esql.expression.function.Example;
3234
import org.elasticsearch.xpack.esql.expression.function.FunctionInfo;
3335
import org.elasticsearch.xpack.esql.expression.function.FunctionType;
34-
import org.elasticsearch.xpack.esql.expression.function.FunctionUtils;
35-
import org.elasticsearch.xpack.esql.expression.function.FunctionUtils.TypeResolutionValidator;
3636
import org.elasticsearch.xpack.esql.expression.function.Param;
3737
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
3838
import org.elasticsearch.xpack.esql.planner.ToAggregator;
@@ -49,8 +49,8 @@
4949
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isNotNull;
5050
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isString;
5151
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isType;
52-
import static org.elasticsearch.xpack.esql.expression.function.FunctionUtils.TypeResolutionValidator.forPostOptimizationValidation;
53-
import static org.elasticsearch.xpack.esql.expression.function.FunctionUtils.TypeResolutionValidator.forPreOptimizationValidation;
52+
import static org.elasticsearch.xpack.esql.expression.Foldables.TypeResolutionValidator.forPostOptimizationValidation;
53+
import static org.elasticsearch.xpack.esql.expression.Foldables.TypeResolutionValidator.forPreOptimizationValidation;
5454

5555
public class Top extends AggregateFunction implements ToAggregator, SurrogateExpression, PostOptimizationVerificationAware {
5656
public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(Expression.class, "Top", Top::new);
@@ -123,7 +123,7 @@ Expression orderField() {
123123
}
124124

125125
private Integer limitValue() {
126-
return FunctionUtils.limitValue(limitField(), sourceText());
126+
return Foldables.limitValue(limitField(), sourceText());
127127
}
128128

129129
private boolean orderValue() {
@@ -181,7 +181,7 @@ protected TypeResolution resolveType() {
181181
* During postOptimizationVerification folding is already done, so we also verify that it is definitively a literal
182182
*/
183183
private TypeResolution resolveTypeLimit() {
184-
return FunctionUtils.resolveTypeLimit(limitField(), sourceText(), forPreOptimizationValidation(limitField()));
184+
return Foldables.resolveTypeLimit(limitField(), sourceText(), forPreOptimizationValidation(limitField()));
185185
}
186186

187187
/**
@@ -238,7 +238,7 @@ public void postOptimizationVerification(Failures failures) {
238238
}
239239

240240
private void postOptimizationVerificationLimit(Failures failures) {
241-
FunctionUtils.resolveTypeLimit(limitField(), sourceText(), forPostOptimizationValidation(limitField(), failures));
241+
Foldables.resolveTypeLimit(limitField(), sourceText(), forPostOptimizationValidation(limitField(), failures));
242242
}
243243

244244
private void postOptimizationVerificationOrder(Failures failures) {

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/FullTextFunction.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@
5656
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.DEFAULT;
5757
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isNotNull;
5858
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isString;
59-
import static org.elasticsearch.xpack.esql.expression.function.FunctionUtils.TypeResolutionValidator.forPostOptimizationValidation;
60-
import static org.elasticsearch.xpack.esql.expression.function.FunctionUtils.TypeResolutionValidator.forPreOptimizationValidation;
61-
import static org.elasticsearch.xpack.esql.expression.function.FunctionUtils.resolveTypeQuery;
59+
import static org.elasticsearch.xpack.esql.expression.Foldables.TypeResolutionValidator.forPostOptimizationValidation;
60+
import static org.elasticsearch.xpack.esql.expression.Foldables.TypeResolutionValidator.forPreOptimizationValidation;
61+
import static org.elasticsearch.xpack.esql.expression.Foldables.resolveTypeQuery;
6262

6363
/**
6464
* Base class for full-text functions that use ES queries to match documents.

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Kql.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@
1616
import org.elasticsearch.xpack.esql.core.querydsl.query.Query;
1717
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
1818
import org.elasticsearch.xpack.esql.core.tree.Source;
19+
import org.elasticsearch.xpack.esql.expression.Foldables;
1920
import org.elasticsearch.xpack.esql.expression.function.Example;
2021
import org.elasticsearch.xpack.esql.expression.function.FunctionAppliesTo;
2122
import org.elasticsearch.xpack.esql.expression.function.FunctionAppliesToLifecycle;
2223
import org.elasticsearch.xpack.esql.expression.function.FunctionInfo;
23-
import org.elasticsearch.xpack.esql.expression.function.FunctionUtils;
2424
import org.elasticsearch.xpack.esql.expression.function.Param;
2525
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
2626
import org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushdownPredicates;
@@ -95,7 +95,7 @@ protected NodeInfo<? extends Expression> info() {
9595

9696
@Override
9797
protected Query translate(LucenePushdownPredicates pushdownPredicates, TranslatorHandler handler) {
98-
return new KqlQuery(source(), FunctionUtils.queryAsString(query(), sourceText()));
98+
return new KqlQuery(source(), Foldables.queryAsString(query(), sourceText()));
9999
}
100100

101101
@Override

0 commit comments

Comments
 (0)