Skip to content

Commit 95f84d2

Browse files
committed
Refactor Greatest function to use evaluator map
This commit refactors the Greatest function to use an evaluator map for better code organization. The changes include: - Added EVALUATOR_MAP for evaluator factory mapping - Updated toEvaluator to use map-based lookup - Added NULL type validation - Updated error messages to use getWriteableName() The refactoring improves code readability and maintainability by: - Replacing if-else chains with a map-based lookup - Centralizing evaluator factory creation - Adding consistent NULL type validation - Using getWriteableName() for error messages Relates to #114036
1 parent dc90751 commit 95f84d2

File tree

1 file changed

+35
-14
lines changed
  • x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional

1 file changed

+35
-14
lines changed

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/Greatest.java

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.common.io.stream.StreamInput;
1313
import org.elasticsearch.common.io.stream.StreamOutput;
1414
import org.elasticsearch.compute.ann.Evaluator;
15+
import org.elasticsearch.compute.operator.EvalOperator;
1516
import org.elasticsearch.compute.operator.EvalOperator.ExpressionEvaluator;
1617
import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
1718
import org.elasticsearch.xpack.esql.core.expression.Expression;
@@ -27,9 +28,15 @@
2728
import org.elasticsearch.xpack.esql.expression.function.scalar.EsqlScalarFunction;
2829
import org.elasticsearch.xpack.esql.expression.function.scalar.multivalue.MvMax;
2930
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
31+
import org.elasticsearch.compute.operator.EvalOperator;
32+
import org.elasticsearch.compute.operator.EvalOperator.ExpressionEvaluator;
3033

3134
import java.io.IOException;
3235
import java.util.List;
36+
import java.util.Map;
37+
import java.util.Set;
38+
import java.util.function.BiFunction;
39+
import java.util.function.Function;
3340
import java.util.stream.Stream;
3441

3542
import static org.elasticsearch.xpack.esql.core.type.DataType.NULL;
@@ -42,6 +49,17 @@ public class Greatest extends EsqlScalarFunction implements OptionalArgument {
4249

4350
private DataType dataType;
4451

52+
private static final Map<DataType, Function<ExpressionEvaluator.Factory[], ExpressionEvaluator.Factory>> EVALUATOR_MAP = Map.of(
53+
DataType.BOOLEAN, factories -> new GreatestBooleanEvaluator.Factory(Source.EMPTY, factories),
54+
DataType.DOUBLE, factories -> new GreatestDoubleEvaluator.Factory(Source.EMPTY, factories),
55+
DataType.INTEGER, factories -> new GreatestIntEvaluator.Factory(Source.EMPTY, factories),
56+
DataType.LONG, factories -> new GreatestLongEvaluator.Factory(Source.EMPTY, factories),
57+
DataType.DATETIME, factories -> new GreatestLongEvaluator.Factory(Source.EMPTY, factories),
58+
DataType.DATE_NANOS, factories -> new GreatestLongEvaluator.Factory(Source.EMPTY, factories),
59+
DataType.IP, factories -> new GreatestBytesRefEvaluator.Factory(Source.EMPTY, factories),
60+
DataType.VERSION, factories -> new GreatestBytesRefEvaluator.Factory(Source.EMPTY, factories)
61+
);
62+
4563
@FunctionInfo(
4664
returnType = { "boolean", "date", "date_nanos", "double", "integer", "ip", "keyword", "long", "version" },
4765
description = "Returns the maximum value from multiple columns. This is similar to <<esql-mv_max>>\n"
@@ -118,6 +136,11 @@ protected TypeResolution resolveType() {
118136
return resolution;
119137
}
120138
}
139+
140+
if (dataType != NULL && !EVALUATOR_MAP.containsKey(dataType) && !DataType.isString(dataType)) {
141+
return new TypeResolution("Cannot use [" + dataType.typeName() + "] with function [" + getWriteableName() + "]");
142+
}
143+
121144
return TypeResolution.TYPE_RESOLVED;
122145
}
123146

@@ -140,26 +163,24 @@ public boolean foldable() {
140163
public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
141164
// force datatype initialization
142165
var dataType = dataType();
166+
if (dataType == DataType.NULL) {
167+
throw EsqlIllegalArgumentException.illegalDataType(dataType);
168+
}
169+
143170
ExpressionEvaluator.Factory[] factories = children().stream()
144171
.map(e -> toEvaluator.apply(new MvMax(e.source(), e)))
145172
.toArray(ExpressionEvaluator.Factory[]::new);
146-
if (dataType == DataType.BOOLEAN) {
147-
return new GreatestBooleanEvaluator.Factory(source(), factories);
148-
}
149-
if (dataType == DataType.DOUBLE) {
150-
return new GreatestDoubleEvaluator.Factory(source(), factories);
151-
}
152-
if (dataType == DataType.INTEGER) {
153-
return new GreatestIntEvaluator.Factory(source(), factories);
154-
}
155-
if (dataType == DataType.LONG || dataType == DataType.DATETIME || dataType == DataType.DATE_NANOS) {
156-
return new GreatestLongEvaluator.Factory(source(), factories);
157-
}
158-
if (DataType.isString(dataType) || dataType == DataType.IP || dataType == DataType.VERSION || dataType == DataType.UNSUPPORTED) {
159173

174+
if (DataType.isString(dataType)) {
160175
return new GreatestBytesRefEvaluator.Factory(source(), factories);
161176
}
162-
throw EsqlIllegalArgumentException.illegalDataType(dataType);
177+
178+
var evaluatorFactory = EVALUATOR_MAP.get(dataType);
179+
if (evaluatorFactory == null) {
180+
throw EsqlIllegalArgumentException.illegalDataType(dataType);
181+
}
182+
183+
return evaluatorFactory.apply(factories);
163184
}
164185

165186
@Evaluator(extraName = "Boolean")

0 commit comments

Comments
 (0)