Skip to content

ESQL: Finish migrating error testing #143322

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

Merged
nik9000 merged 3 commits into from
Mar 2, 2026
Merged

ESQL: Finish migrating error testing #143322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4a081d8
ESQL: Finish migrating error testing
nik9000 Feb 28, 2026
a2b9a82
Merge branch 'main' into esql_remove_errors_for_cases
nik9000 Feb 28, 2026
b877d04
Merge branch 'main' into esql_remove_errors_for_cases
nik9000 Feb 28, 2026
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
3 changes: 1 addition & 2 deletions ...in/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Top.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.SECOND;
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.THIRD;
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isNotNull;
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isString;
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isType;
import static org.elasticsearch.xpack.esql.expression.Foldables.TypeResolutionValidator.forPostOptimizationValidation;
import static org.elasticsearch.xpack.esql.expression.Foldables.TypeResolutionValidator.forPreOptimizationValidation;
Expand Down Expand Up @@ -189,7 +188,7 @@ protected TypeResolution resolveType() {
).and(isNotNull(limitField(), sourceText(), SECOND))
.and(isType(limitField(), dt -> dt == DataType.INTEGER, sourceText(), SECOND, "integer"))
.and(isNotNull(orderField(), sourceText(), THIRD))
.and(isString(orderField(), sourceText(), THIRD));
.and(isType(orderField(), dt -> dt == DataType.KEYWORD, sourceText(), THIRD, "keyword"));
Copy link
Copy Markdown
Member Author

@nik9000 nik9000 Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wants a constant string and those are always keyword.

if (outputField() != null) {
typeResolution = typeResolution.and(
isType(
Expand Down
13 changes: 1 addition & 12 deletions ...st/java/org/elasticsearch/xpack/esql/expression/function/AbstractAggregationTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,7 @@ protected static Iterable<Object[]> parameterSuppliersFromTypedDataWithDefaultCh
List<TestCaseSupplier> suppliers,
PositionalErrorMessageSupplier positionalErrorMessageSupplier
) {
return parameterSuppliersFromTypedData(
errorsForCasesWithoutExamples(withNoRowsExpectingNull(randomizeBytesRefsOffset(suppliers)), positionalErrorMessageSupplier)
);
return parameterSuppliersFromTypedData(withNoRowsExpectingNull(randomizeBytesRefsOffset(suppliers)));
}

protected static Iterable<Object[]> parameterSuppliersFromTypedDataWithDefaultChecks(List<TestCaseSupplier> suppliers) {
Expand Down Expand Up @@ -113,7 +111,6 @@ protected static List<TestCaseSupplier> withNoRowsExpectingNull(List<TestCaseSup
nullValue(),
null,
null,
testCase.getExpectedTypeError(),
null,
null,
null,
Expand Down Expand Up @@ -522,10 +519,6 @@ private Expression resolveExpression(Expression expression) {
valuesString = valuesString.substring(0, 200) + "...";
}
logger.info("Test Values: " + valuesString);
if (testCase.getExpectedTypeError() != null) {
assertTypeResolutionFailure(expression);
return null;
}
assertThat(expression.dataType(), equalTo(testCase.expectedType()));
expression = resolveSurrogates(expression);

Expand Down Expand Up @@ -605,10 +598,6 @@ private List<Integer> intermediaryInputChannels(int intermediaryStates, int offs
* </p>
*/
private Expression resolveSurrogates(Expression expression) {
if (testCase.getExpectedTypeError() != null) {
return expression;
}

// Run agg surrogates twice
// This simulates the double aggs surrogation in LogicalPlanOptimizer
for (int i = 0; i < 2; i++) {
Expand Down
126 changes: 6 additions & 120 deletions .../test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@
import java.util.stream.Stream;

import static org.elasticsearch.compute.data.BlockUtils.toJavaObject;
import static org.elasticsearch.xpack.esql.EsqlTestUtils.randomLiteral;
import static org.elasticsearch.xpack.esql.EsqlTestUtils.unboundLogicalOptimizerContext;
import static org.elasticsearch.xpack.esql.SerializationTestUtils.assertSerialization;
import static org.elasticsearch.xpack.esql.SerializationTestUtils.serializeDeserialize;
Expand Down Expand Up @@ -126,7 +125,7 @@ protected static Iterable<Object[]> parameterSuppliersFromTypedData(List<TestCas
* </p>
*
* @param entirelyNullPreservesType should a test case that only contains parameters
* with the {@code null} type keep it’s expected type?
* with the {@code null} type keep it's expected type?
* This is <strong>mostly</strong> going to be {@code true}
* except for functions that base their type entirely
* on input types like {@link Greatest} or {@link Coalesce}.
Expand Down Expand Up @@ -164,7 +163,7 @@ protected static List<TestCaseSupplier> anyNullIsNull(
*
* Also, if this was the first time we saw the signature we copy it
* *again*, replacing the argument with null, but annotating the
* argument’s type as `null` explicitly.
* argument's type as `null` explicitly.
*/
Set<List<DataType>> uniqueSignatures = new HashSet<>();
for (TestCaseSupplier original : testCaseSuppliers) {
Expand All @@ -190,7 +189,6 @@ protected static List<TestCaseSupplier> anyNullIsNull(
nullValue(),
null,
null,
oc.getExpectedTypeError(),
null,
null,
null,
Expand Down Expand Up @@ -224,7 +222,6 @@ protected static List<TestCaseSupplier> anyNullIsNull(
nullValue(),
null,
null,
oc.getExpectedTypeError(),
null,
null,
null,
Expand Down Expand Up @@ -254,19 +251,6 @@ public interface PositionalErrorMessageSupplier {
String apply(Set<DataType> validForPosition, int position);
}

/**
* Adds test cases containing unsupported parameter types that assert
* that they throw type errors.
* @deprecated make a subclass of {@link ErrorsForCasesWithoutExamplesTestCase} instead
*/
@Deprecated
protected static List<TestCaseSupplier> errorsForCasesWithoutExamples(
List<TestCaseSupplier> testCaseSuppliers,
PositionalErrorMessageSupplier positionalErrorMessageSupplier
) {
return errorsForCasesWithoutExamples(testCaseSuppliers, (i, v, t) -> typeErrorMessage(i, v, t, positionalErrorMessageSupplier));
}

/**
* Build the expected error message for an invalid type signature.
*/
Expand Down Expand Up @@ -294,98 +278,18 @@ protected static String typeErrorMessage(
return ordinal + "argument of [source] must be [" + expectedTypeString + "], found value [" + name + "] type [" + name + "]";
}

@FunctionalInterface
protected interface TypeErrorMessageSupplier {
String apply(boolean includeOrdinal, List<Set<DataType>> validPerPosition, List<DataType> types);
}

/**
* @deprecated make a subclass of {@link ErrorsForCasesWithoutExamplesTestCase} instead
*/
@Deprecated
protected static List<TestCaseSupplier> errorsForCasesWithoutExamples(
List<TestCaseSupplier> testCaseSuppliers,
TypeErrorMessageSupplier typeErrorMessageSupplier
) {
List<TestCaseSupplier> suppliers = new ArrayList<>(testCaseSuppliers.size());
suppliers.addAll(testCaseSuppliers);

Set<List<DataType>> valid = testCaseSuppliers.stream().map(TestCaseSupplier::types).collect(Collectors.toSet());
List<Set<DataType>> validPerPosition = validPerPosition(valid);

testCaseSuppliers.stream()
.map(s -> s.types().size())
.collect(Collectors.toSet())
.stream()
.flatMap(AbstractFunctionTestCase::allPermutations)
.filter(types -> valid.contains(types) == false)
/*
* Skip any cases with more than one null. Our tests don't generate
* the full combinatorial explosions of all nulls - just a single null.
* Hopefully <null>, <null> cases will function the same as <null>, <valid>
* cases.
*/.filter(types -> types.stream().filter(t -> t == DataType.NULL).count() <= 1)
.map(types -> typeErrorSupplier(validPerPosition.size() != 1, validPerPosition, types, typeErrorMessageSupplier))
.forEach(suppliers::add);
return suppliers;
}

private static List<DataType> append(List<DataType> orig, DataType extra) {
List<DataType> longer = new ArrayList<>(orig.size() + 1);
longer.addAll(orig);
longer.add(extra);
return longer;
}

protected static TestCaseSupplier typeErrorSupplier(
boolean includeOrdinal,
List<Set<DataType>> validPerPosition,
List<DataType> types,
PositionalErrorMessageSupplier errorMessageSupplier
) {
return typeErrorSupplier(includeOrdinal, validPerPosition, types, (o, v, t) -> typeErrorMessage(o, v, t, errorMessageSupplier));
}

/**
* Build a test case that asserts that the combination of parameter types is an error.
* @deprecated use an extension of {@link ErrorsForCasesWithoutExamplesTestCase}
*/
@Deprecated
protected static TestCaseSupplier typeErrorSupplier(
boolean includeOrdinal,
List<Set<DataType>> validPerPosition,
List<DataType> types,
TypeErrorMessageSupplier errorMessageSupplier
) {
return new TestCaseSupplier(
"type error for " + TestCaseSupplier.nameFromTypes(types),
types,
() -> TestCaseSupplier.TestCase.typeError(
types.stream().map(type -> new TestCaseSupplier.TypedData(randomLiteral(type).value(), type, type.typeName())).toList(),
errorMessageSupplier.apply(includeOrdinal, validPerPosition, types)
)
);
}

static List<Set<DataType>> validPerPosition(Set<List<DataType>> valid) {
int max = valid.stream().mapToInt(List::size).max().getAsInt();
List<Set<DataType>> result = new ArrayList<>(max);
for (int i = 0; i < max; i++) {
result.add(new HashSet<>());
}
for (List<DataType> signature : valid) {
for (int i = 0; i < signature.size(); i++) {
result.get(i).add(signature.get(i));
}
}
return result;
}

protected static Stream<List<DataType>> allPermutations(int argumentCount) {
if (argumentCount == 0) {
return Stream.of(List.of());
}
if (argumentCount > 3) {
if (argumentCount > 4) {
throw new IllegalArgumentException("would generate too many combinations");
}
Stream<List<DataType>> stream = validFunctionParameters().map(List::of);
Expand Down Expand Up @@ -428,15 +332,15 @@ public static Stream<DataType> validFunctionParameters() {
if (DataType.UNDER_CONSTRUCTION.contains(t)) {
/*
* Types under construction aren't checked because we're actively
* adding support for them to functions. That’s *why* they are
* adding support for them to functions. That's *why* they are
* under construction.
*/
return false;
}
if (t.isCounter()) {
/*
* For now, we're assuming no functions take counters
* as parameters. That’s not true - some do. But we'll
* as parameters. That's not true - some do. But we'll
* need to update the tests to handle that.
*/
return false;
Expand Down Expand Up @@ -934,14 +838,9 @@ protected final void assertTestCaseResultAndWarnings(Object originalResult) {
}
}

protected final void assertTypeResolutionFailure(Expression expression) {
assertTrue("expected unresolved", expression.typeResolved().unresolved());
assertThat(expression.typeResolved().message(), equalTo(testCase.getExpectedTypeError()));
}

private static Class<?> classGeneratingSignatures = null;
/**
* Unique signatures in this test’s parameters.
* Unique signatures in this test's parameters.
*/
private static Set<DocsV3Support.TypeSignature> signatures;

Expand All @@ -964,9 +863,6 @@ public static Set<DocsV3Support.TypeSignature> signatures(Class<?> testClass) {
TestCaseSupplier tcs = (TestCaseSupplier) ((Object[]) p)[0];
try {
TestCaseSupplier.TestCase tc = tcs.get();
if (tc.getExpectedTypeError() != null) {
continue;
}
if (tc.getData().stream().anyMatch(t -> t.type() == DataType.NULL)) {
continue;
}
Expand Down Expand Up @@ -1055,16 +951,6 @@ public void allMemoryReleased() {
}
}

/**
* Returns true if the current test case is for an aggregation function.
* <p>
* This method requires reflection, as it’s called from a static context (@AfterClass documentation rendering).
* </p>
*/
private static boolean isAggregation() {
return AbstractAggregationTestCase.class.isAssignableFrom(getTestClass());
}

/**
* Should this particular signature be hidden from the docs even though we test it?
*/
Expand Down
26 changes: 1 addition & 25 deletions ...java/org/elasticsearch/xpack/esql/expression/function/AbstractScalarFunctionTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,6 @@ public final void testEvaluate() {
assumeTrue("Can't build evaluator", testCase.canBuildEvaluator());
boolean readFloating = randomBoolean();
Expression expression = readFloating ? buildDeepCopyOfFieldExpression(testCase) : buildFieldExpression(testCase);
if (testCase.getExpectedTypeError() != null) {
assertTypeResolutionFailure(expression);
return;
}
logger.info(
"Test Values: " + testCase.getData().stream().map(TestCaseSupplier.TypedData::toString).collect(Collectors.joining(","))
);
Expand Down Expand Up @@ -229,10 +225,6 @@ protected Matcher<Object> allNullsMatcher() {

private void testEvaluateBlock(BlockFactory inputBlockFactory, DriverContext context, boolean insertNulls) {
Expression expression = randomBoolean() ? buildDeepCopyOfFieldExpression(testCase) : buildFieldExpression(testCase);
if (testCase.getExpectedTypeError() != null) {
assertTypeResolutionFailure(expression);
return;
}
assumeTrue("Can't build evaluator", testCase.canBuildEvaluator());
int positions = between(1, 1024);
List<TestCaseSupplier.TypedData> data = testCase.getData();
Expand Down Expand Up @@ -293,10 +285,6 @@ private void testEvaluateBlock(BlockFactory inputBlockFactory, DriverContext con

public final void testEvaluateInManyThreads() throws ExecutionException, InterruptedException {
Expression expression = buildFieldExpression(testCase);
if (testCase.getExpectedTypeError() != null) {
assertTypeResolutionFailure(expression);
return;
}
assumeTrue("Can't build evaluator", testCase.canBuildEvaluator());
int count;
Set<DataType> complexTypes = Set.of(DataType.EXPONENTIAL_HISTOGRAM);
Expand Down Expand Up @@ -341,10 +329,6 @@ public final void testEvaluateInManyThreads() throws ExecutionException, Interru

public final void testEvaluatorToString() {
Expression expression = buildFieldExpression(testCase);
if (testCase.getExpectedTypeError() != null) {
assertTypeResolutionFailure(expression);
return;
}
assumeTrue("Can't build evaluator", testCase.canBuildEvaluator());
var factory = evaluator(expression);
try (ExpressionEvaluator ev = factory.get(driverContext())) {
Expand All @@ -357,10 +341,6 @@ public final void testEvaluatorToString() {

public final void testFactoryToString() {
Expression expression = buildFieldExpression(testCase);
if (testCase.getExpectedTypeError() != null) {
assertTypeResolutionFailure(expression);
return;
}
assumeTrue("Can't build evaluator", testCase.canBuildEvaluator());
var factory = evaluator(buildFieldExpression(testCase));
if (testCase.getExpectedBuildEvaluatorWarnings() != null) {
Expand All @@ -371,10 +351,6 @@ public final void testFactoryToString() {

public void testFold() {
Expression expression = buildLiteralExpression(testCase);
if (testCase.getExpectedTypeError() != null) {
assertTypeResolutionFailure(expression);
return;
}
assertFalse("expected resolved", expression.typeResolved().unresolved());
if (expression instanceof SurrogateExpression s) {
Expression surrogate = s.surrogate();
Expand Down Expand Up @@ -421,7 +397,7 @@ protected static List<TestCaseSupplier> failureForCasesWithoutExamples(List<Test
.map(s -> s.types().size())
.collect(Collectors.toSet())
.stream()
.flatMap(count -> allPermutations(count))
.flatMap(AbstractFunctionTestCase::allPermutations)
.filter(types -> valid.contains(types) == false)
.map(types -> new TestCaseSupplier("type error for " + TestCaseSupplier.nameFromTypes(types), types, () -> {
throw new IllegalStateException("must implement a case for " + types);
Expand Down
Loading