Skip to content

[9.0] ESQL: Refactor Greatest and Least functions to use evaluator map (#114036) #128429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

[9.0] ESQL: Refactor Greatest and Least functions to use evaluator map (#114036) #128429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
95f84d2
Refactor Greatest function to use evaluator map
shinsj4653 May 25, 2025
866d650
Refactor Least function to use evaluator map
shinsj4653 May 25, 2025
6f8614f
Add tests for Greatest and Least functions
shinsj4653 May 25, 2025
23a28c2
Fix PR number in commit messages (#128429)
shinsj4653 May 25, 2025
6e6f92c
Update docs/changelog/128429.yaml
shinsj4653 May 25, 2025
5299b22
ESQL: Refactor Greatest and Least functions to use switch expressions…
shinsj4653 Sep 27, 2025
ff3dd9c
ESQL: Add type validation tests for Greatest and Least functions- Add…
shinsj4653 Sep 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog/128429.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 128429
summary: [ESQL] Refactor Greatest and Least functions to use evaluator map
area: ES|QL
type: bug
issues:
- 114036
47 changes: 33 additions & 14 deletions ...in/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/Greatest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.compute.ann.Evaluator;
import org.elasticsearch.compute.operator.EvalOperator;
import org.elasticsearch.compute.operator.EvalOperator.ExpressionEvaluator;
import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
import org.elasticsearch.xpack.esql.core.expression.Expression;
Expand All @@ -27,6 +28,8 @@
import org.elasticsearch.xpack.esql.expression.function.scalar.EsqlScalarFunction;
import org.elasticsearch.xpack.esql.expression.function.scalar.multivalue.MvMax;
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
import org.elasticsearch.compute.operator.EvalOperator;
import org.elasticsearch.compute.operator.EvalOperator.ExpressionEvaluator;

import java.io.IOException;
import java.util.List;
Expand All @@ -42,6 +45,7 @@ public class Greatest extends EsqlScalarFunction implements OptionalArgument {

private DataType dataType;


@FunctionInfo(
returnType = { "boolean", "date", "date_nanos", "double", "integer", "ip", "keyword", "long", "version" },
description = "Returns the maximum value from multiple columns. This is similar to <<esql-mv_max>>\n"
Expand Down Expand Up @@ -118,6 +122,11 @@ protected TypeResolution resolveType() {
return resolution;
}
}

if (dataType != NULL && !isSupportedDataType(dataType) && !DataType.isString(dataType)) {
return new TypeResolution("Cannot use [" + dataType.typeName() + "] with function [" + getWriteableName() + "]");
}

return TypeResolution.TYPE_RESOLVED;
}

Expand All @@ -140,26 +149,29 @@ public boolean foldable() {
public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
// force datatype initialization
var dataType = dataType();
if (dataType == DataType.NULL) {
throw EsqlIllegalArgumentException.illegalDataType(dataType);
}

ExpressionEvaluator.Factory[] factories = children().stream()
.map(e -> toEvaluator.apply(new MvMax(e.source(), e)))
.toArray(ExpressionEvaluator.Factory[]::new);
if (dataType == DataType.BOOLEAN) {
return new GreatestBooleanEvaluator.Factory(source(), factories);
}
if (dataType == DataType.DOUBLE) {
return new GreatestDoubleEvaluator.Factory(source(), factories);
}
if (dataType == DataType.INTEGER) {
return new GreatestIntEvaluator.Factory(source(), factories);
}
if (dataType == DataType.LONG || dataType == DataType.DATETIME || dataType == DataType.DATE_NANOS) {
return new GreatestLongEvaluator.Factory(source(), factories);
}
if (DataType.isString(dataType) || dataType == DataType.IP || dataType == DataType.VERSION || dataType == DataType.UNSUPPORTED) {

if (DataType.isString(dataType)) {
return new GreatestBytesRefEvaluator.Factory(source(), factories);
}
throw EsqlIllegalArgumentException.illegalDataType(dataType);

return switch (dataType) {
case BOOLEAN -> new GreatestBooleanEvaluator.Factory(source(), factories);
case DOUBLE -> new GreatestDoubleEvaluator.Factory(source(), factories);
case INTEGER -> new GreatestIntEvaluator.Factory(source(), factories);
case LONG -> new GreatestLongEvaluator.Factory(source(), factories);
case DATETIME -> new GreatestLongEvaluator.Factory(source(), factories);
case DATE_NANOS -> new GreatestLongEvaluator.Factory(source(), factories);
case IP -> new GreatestBytesRefEvaluator.Factory(source(), factories);
case VERSION -> new GreatestBytesRefEvaluator.Factory(source(), factories);
default -> throw EsqlIllegalArgumentException.illegalDataType(dataType);
};
}

@Evaluator(extraName = "Boolean")
Expand Down Expand Up @@ -208,5 +220,12 @@ static double process(double[] values) {
return max;
}

private static boolean isSupportedDataType(DataType dataType) {
return switch (dataType) {
case BOOLEAN, DOUBLE, INTEGER, LONG, DATETIME, DATE_NANOS, IP, VERSION -> true;
default -> false;
};
}

// TODO unsigned long
}
44 changes: 30 additions & 14 deletions .../main/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/Least.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.compute.ann.Evaluator;
import org.elasticsearch.compute.operator.EvalOperator;
import org.elasticsearch.compute.operator.EvalOperator.ExpressionEvaluator;
import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
import org.elasticsearch.xpack.esql.core.expression.Expression;
Expand Down Expand Up @@ -42,6 +43,7 @@ public class Least extends EsqlScalarFunction implements OptionalArgument {

private DataType dataType;


@FunctionInfo(
returnType = { "boolean", "date", "date_nanos", "double", "integer", "ip", "keyword", "long", "version" },
description = "Returns the minimum value from multiple columns. "
Expand Down Expand Up @@ -116,6 +118,11 @@ protected TypeResolution resolveType() {
return resolution;
}
}

if (dataType != NULL && !isSupportedDataType(dataType) && !DataType.isString(dataType)) {
return new TypeResolution("Cannot use [" + dataType.typeName() + "] with function [" + getWriteableName() + "]");
}

return TypeResolution.TYPE_RESOLVED;
}

Expand All @@ -138,27 +145,29 @@ public boolean foldable() {
public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
// force datatype initialization
var dataType = dataType();
if (dataType == DataType.NULL) {
throw EsqlIllegalArgumentException.illegalDataType(dataType);
}

ExpressionEvaluator.Factory[] factories = children().stream()
.map(e -> toEvaluator.apply(new MvMin(e.source(), e)))
.toArray(ExpressionEvaluator.Factory[]::new);
if (dataType == DataType.BOOLEAN) {
return new LeastBooleanEvaluator.Factory(source(), factories);
}
if (dataType == DataType.DOUBLE) {
return new LeastDoubleEvaluator.Factory(source(), factories);
}
if (dataType == DataType.INTEGER) {
return new LeastIntEvaluator.Factory(source(), factories);
}
if (dataType == DataType.LONG || dataType == DataType.DATETIME || dataType == DataType.DATE_NANOS) {
return new LeastLongEvaluator.Factory(source(), factories);
}
if (DataType.isString(dataType) || dataType == DataType.IP || dataType == DataType.VERSION || dataType == DataType.UNSUPPORTED) {

if (DataType.isString(dataType)) {
return new LeastBytesRefEvaluator.Factory(source(), factories);
}
throw EsqlIllegalArgumentException.illegalDataType(dataType);

return switch (dataType) {
case BOOLEAN -> new LeastBooleanEvaluator.Factory(source(), factories);
case DOUBLE -> new LeastDoubleEvaluator.Factory(source(), factories);
case INTEGER -> new LeastIntEvaluator.Factory(source(), factories);
case LONG -> new LeastLongEvaluator.Factory(source(), factories);
case DATETIME -> new LeastLongEvaluator.Factory(source(), factories);
case DATE_NANOS -> new LeastLongEvaluator.Factory(source(), factories);
case IP -> new LeastBytesRefEvaluator.Factory(source(), factories);
case VERSION -> new LeastBytesRefEvaluator.Factory(source(), factories);
default -> throw EsqlIllegalArgumentException.illegalDataType(dataType);
};
}

@Evaluator(extraName = "Boolean")
Expand Down Expand Up @@ -206,4 +215,11 @@ static double process(double[] values) {
}
return min;
}

private static boolean isSupportedDataType(DataType dataType) {
return switch (dataType) {
case BOOLEAN, DOUBLE, INTEGER, LONG, DATETIME, DATE_NANOS, IP, VERSION -> true;
default -> false;
};
}
}
35 changes: 35 additions & 0 deletions x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2407,6 +2407,41 @@ public void testMultiMatchFunctionIsNotNullable() {
);
}

public void testGreatestLeastWithUnsupportedDataTypes() {
// Test Greatest function with unsupported data types using ROW and type conversion
assertEquals(
"1:64: Cannot use [geo_point] with function [Greatest]",
error("row wkt = \"POINT(1 1)\" | eval geopoint = to_geopoint(wkt), x = greatest(geopoint)")
);

assertEquals(
"1:71: Cannot use [cartesian_point] with function [Greatest]",
error("row wkt = \"POINT(1 1)\" | eval cartpoint = to_cartesianpoint(wkt), x = greatest(cartpoint)")
);

// Test Least function with unsupported data types
assertEquals(
"1:64: Cannot use [geo_point] with function [Least]",
error("row wkt = \"POINT(1 1)\" | eval geopoint = to_geopoint(wkt), x = least(geopoint)")
);

assertEquals(
"1:71: Cannot use [cartesian_point] with function [Least]",
error("row wkt = \"POINT(1 1)\" | eval cartpoint = to_cartesianpoint(wkt), x = least(cartpoint)")
);

// Test with mixed supported and unsupported types
assertEquals(
"1:73: second argument of [greatest(num, geopoint)] must be [integer], found value [geopoint] type [geo_point]",
error("row wkt = \"POINT(1 1)\", num = 1 | eval geopoint = to_geopoint(wkt), x = greatest(num, geopoint)")
);

assertEquals(
"1:73: second argument of [least(num, geopoint)] must be [integer], found value [geopoint] type [geo_point]",
error("row wkt = \"POINT(1 1)\", num = 1 | eval geopoint = to_geopoint(wkt), x = least(num, geopoint)")
);
}

public void testMultiMatchWithNonIndexedColumnCurrentlyUnsupported() {
assertEquals(
"1:78: [MultiMatch] function cannot operate on [initial], which is not a field from an index mapping",
Expand Down