From e534c8bd6d798208fd2f157268f9390d524a5d0f Mon Sep 17 00:00:00 2001 From: cdelgado Date: Mon, 8 Sep 2025 19:30:38 +0200 Subject: [PATCH 01/20] First version --- .../xpack/esql/core/type/DataType.java | 77 +++++++----- .../xpack/esql/plugin/KnnFunctionIT.java | 33 +++++ .../xpack/esql/action/EsqlCapabilities.java | 7 +- .../xpack/esql/action/RequestXContent.java | 57 +++++++-- .../esql/action/EsqlQueryRequestTests.java | 116 +++++++++++++++++- .../xpack/esql/analysis/AnalyzerTests.java | 30 ++++- 6 files changed, 270 insertions(+), 50 deletions(-) diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java index 3c36884874454..0e5149ec5507c 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java @@ -22,6 +22,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Comparator; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Optional; @@ -456,41 +457,51 @@ public static DataType fromEs(String name) { } public static DataType fromJava(Object value) { - if (value == null) { - return NULL; - } - if (value instanceof Integer) { - return INTEGER; - } - if (value instanceof Long) { - return LONG; - } - if (value instanceof BigInteger) { - return UNSIGNED_LONG; - } - if (value instanceof Boolean) { - return BOOLEAN; - } - if (value instanceof Double) { - return DOUBLE; - } - if (value instanceof Float) { - return FLOAT; - } - if (value instanceof Byte) { - return BYTE; - } - if (value instanceof Short) { - return SHORT; - } - if (value instanceof ZonedDateTime) { - return DATETIME; - } - if (value instanceof String || value instanceof Character || value instanceof BytesRef) { - return KEYWORD; + switch (value) { + case null -> { + return NULL; + } + case Integer i -> { + return INTEGER; + } + case Long l -> { + return LONG; + } + case BigInteger bigInteger -> { + return UNSIGNED_LONG; + } + case Boolean b -> { + return BOOLEAN; + } + case Double v -> { + return DOUBLE; + } + case Float v -> { + return FLOAT; + } + case Byte b -> { + return BYTE; + } + case Short i -> { + return SHORT; + } + case ZonedDateTime zonedDateTime -> { + return DATETIME; + } + case List list -> { + if (list.isEmpty()) { + return null; + } + return fromJava(list.getFirst()); + } + default -> { + if (value instanceof String || value instanceof Character || value instanceof BytesRef) { + return KEYWORD; + } + return null; + } } - return null; } public static boolean isUnsupported(DataType from) { diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java index 70356cd36f34e..ed9e8f263e2f9 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java @@ -22,6 +22,11 @@ import org.elasticsearch.xpack.esql.VerificationException; import org.elasticsearch.xpack.esql.action.AbstractEsqlIntegTestCase; import org.elasticsearch.xpack.esql.action.EsqlCapabilities; +import org.elasticsearch.xpack.esql.action.EsqlQueryRequest; +import org.elasticsearch.xpack.esql.core.type.DataType; +import org.elasticsearch.xpack.esql.parser.ParserUtils; +import org.elasticsearch.xpack.esql.parser.QueryParam; +import org.elasticsearch.xpack.esql.parser.QueryParams; import org.junit.Before; import java.io.IOException; @@ -129,6 +134,34 @@ public void testKnnOptions() { } } + public void testDenseVectorQueryParams() { + assumeTrue("multi values for query params capability must be available", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled()); + + float[] queryVector = new float[numDims]; + Arrays.fill(queryVector, 0); + EsqlQueryRequest queryRequest = new EsqlQueryRequest(); + QueryParams queryParams = new QueryParams(List.of(new QueryParam("queryVector", Arrays.asList(queryVector), DataType.INTEGER, + ParserUtils.ParamClassification.PATTERN))); + + queryRequest.params(queryParams); + + var query = String.format(Locale.ROOT, """ + FROM test METADATA _score + | WHERE knn(vector, %s) OR id > 100 + | KEEP id, _score, vector + | SORT _score DESC + | LIMIT 5 + """, Arrays.toString(queryVector)); + + try (var resp = run(query)) { + assertColumnNames(resp.columns(), List.of("id", "_score", "vector")); + assertColumnTypes(resp.columns(), List.of("integer", "double", "dense_vector")); + + List> valuesList = EsqlTestUtils.getValuesList(resp); + assertEquals(5, valuesList.size()); + } + } + public void testKnnNonPushedDown() { float[] queryVector = new float[numDims]; Arrays.fill(queryVector, 0.0f); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index 36ee0536e393b..a1b29996460d8 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -1442,7 +1442,12 @@ public enum Cap { /** * FORK with remote indices */ - ENABLE_FORK_FOR_REMOTE_INDICES(Build.current().isSnapshot()); + ENABLE_FORK_FOR_REMOTE_INDICES(Build.current().isSnapshot()), + + /** + * Multivalued query parameters + */ + QUERY_PARAMS_MULTI_VALUES(Build.current().isSnapshot()); private final boolean enabled; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java index da5c6c761ae93..375d52670314a 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java @@ -167,7 +167,6 @@ private static QueryParams parseParams(XContentParser p) throws IOException { } for (Map.Entry entry : param.fields.entrySet()) { ParserUtils.ParamClassification classification = null; - paramValue = null; String paramName = entry.getKey(); checkParamNameValidity(paramName, errors, loc); @@ -183,18 +182,52 @@ private static QueryParams parseParams(XContentParser p) throws IOException { } else {// parameter specifies as a value only paramValue = entry.getValue(); classification = VALUE; + if (paramValue instanceof List) { + type = null; + List values = new ArrayList<>(); + for (Object currentValue : (List) paramValue) { + checkParamValueValidity(classification, currentValue, loc, errors); + DataType currentType = DataType.fromJava(currentValue); + if (currentType == null) { + errors.add( + new XContentParseException( + loc, + entry + " is not supported as a parameter" + ) + ); + break; + } else if ((type != null) && (type != currentType)) { + errors.add(new XContentParseException( + loc, + paramName + " parameter has values from different types, found " + type + " and " + + currentType + )); + break; + } + type = currentType; + values.add(currentValue); + } + currentParam = new QueryParam( + paramName, + values, + (classification == VALUE) ? type : DataType.NULL, + classification + ); + namedParams.add(currentParam); + } else { + type = DataType.fromJava(paramValue); + if (type == null) { + errors.add(new XContentParseException(loc, entry + " is not supported as a parameter")); + } + currentParam = new QueryParam( + paramName, + paramValue, + (classification == VALUE) ? type : DataType.NULL, + classification + ); + namedParams.add(currentParam); + } } - type = DataType.fromJava(paramValue); - if (type == null) { - errors.add(new XContentParseException(loc, entry + " is not supported as a parameter")); - } - currentParam = new QueryParam( - paramName, - paramValue, - (classification == VALUE) ? type : DataType.NULL, - classification - ); - namedParams.add(currentParam); } } else { paramValue = null; diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index 00ac4837a1fe7..7d520a4f465f2 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -35,6 +35,8 @@ import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.esql.Column; import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.core.type.DataType; +import org.elasticsearch.xpack.esql.parser.ParserUtils; import org.elasticsearch.xpack.esql.parser.ParsingException; import org.elasticsearch.xpack.esql.parser.QueryParam; import org.elasticsearch.xpack.esql.parser.QueryParams; @@ -42,6 +44,7 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Locale; @@ -104,7 +107,10 @@ public void testNamedParams() throws IOException { ,"params":[ {"n1" : "8.15.0"}, { "n2" : 0.05}, {"n3" : -799810013}, {"n4" : "127.0.0.1"}, {"n5" : "esql"}, {"n_6" : null}, {"n7_" : false}, {"_n1" : "8.15.0"}, { "__n2" : 0.05}, {"__3" : -799810013}, - {"__4n" : "127.0.0.1"}, {"_n5" : "esql"}, {"_n6" : null}, {"_n7" : false}] }"""; + {"__4n" : "127.0.0.1"}, {"_n5" : "esql"}, {"_n6" : null}, {"_n7" : false}, + {"_n8": ["8.15.0", "8.19.0"]}, {"_n9": ["x", "y"]}, {"_n10": [true, false]}, {"_n11": [1.0, 1.1, 1.2]}, + {"_n12": [-799810013, 0, 799810013]}, {"_n13": [null, null, null]} + ] }"""; List params = List.of( paramAsConstant("n1", "8.15.0"), @@ -120,7 +126,56 @@ public void testNamedParams() throws IOException { paramAsConstant("__4n", "127.0.0.1"), paramAsConstant("_n5", "esql"), paramAsConstant("_n6", null), - paramAsConstant("_n7", false) + paramAsConstant("_n7", false), + new QueryParam("_n8", List.of("8.15.0", "8.19.0"), KEYWORD, ParserUtils.ParamClassification.VALUE), + new QueryParam("_n9", List.of("x", "y"), KEYWORD, ParserUtils.ParamClassification.VALUE), + new QueryParam("_n10", List.of(true, false), BOOLEAN, ParserUtils.ParamClassification.VALUE), + new QueryParam("_n11", List.of(1.0, 1.1, 1.2), DOUBLE, ParserUtils.ParamClassification.VALUE), + new QueryParam("_n12", List.of(-799810013, 0, 799810013), DataType.INTEGER, ParserUtils.ParamClassification.VALUE), + new QueryParam("_n13", Arrays.asList(null, null, null), NULL, ParserUtils.ParamClassification.VALUE) + // TODO add mixed null values, or check all elements, and separate into a new method + ); + String json = String.format(Locale.ROOT, """ + { + "query": "%s", + "columnar": %s, + "locale": "%s", + "filter": %s + %s""", query, columnar, locale.toLanguageTag(), filter, paramsString); + + EsqlQueryRequest request = parseEsqlQueryRequestSync(json); + + assertEquals(query, request.query()); + assertEquals(columnar, request.columnar()); + assertEquals(locale.toLanguageTag(), request.locale().toLanguageTag()); + assertEquals(locale, request.locale()); + assertEquals(filter, request.filter()); + assertEquals(params.size(), request.params().size()); + + for (int i = 0; i < request.params().size(); i++) { + assertEquals(params.get(i), request.params().get(i + 1)); + } + } + + public void testNamedMultivaluedParams() throws IOException { + String query = randomAlphaOfLengthBetween(1, 100); + boolean columnar = randomBoolean(); + Locale locale = randomLocale(random()); + QueryBuilder filter = randomQueryBuilder(); + + String paramsString = """ + ,"params":[ + {"_n1": ["8.15.0", "8.19.0"]}, {"_n2": ["x", "y"]}, {"_n3": [true, false]}, {"_n4": [1.0, 1.1, 1.2]}, + {"_n5": [-799810013, 0, 799810013]}, {"_n6": [null, null, null]}, + ] }"""; + + List params = List.of( + new QueryParam("_n1", List.of("8.15.0", "8.19.0"), KEYWORD, ParserUtils.ParamClassification.VALUE), + new QueryParam("_n2", List.of("x", "y"), KEYWORD, ParserUtils.ParamClassification.VALUE), + new QueryParam("_n3", List.of(true, false), BOOLEAN, ParserUtils.ParamClassification.VALUE), + new QueryParam("_n4", List.of(1.0, 1.1, 1.2), DOUBLE, ParserUtils.ParamClassification.VALUE), + new QueryParam("_n5", List.of(-799810013, 0, 799810013), DataType.INTEGER, ParserUtils.ParamClassification.VALUE), + new QueryParam("_n6", Arrays.asList(null, null, null), NULL, ParserUtils.ParamClassification.VALUE) ); String json = String.format(Locale.ROOT, """ { @@ -256,6 +311,63 @@ public void testInvalidParams() throws IOException { ); } + public void testInvalidMultivaluedParams() throws IOException { + String query = randomAlphaOfLengthBetween(1, 100); + boolean columnar = randomBoolean(); + Locale locale = randomLocale(random()); + QueryBuilder filter = randomQueryBuilder(); + + // invalid named parameter for multivalued constants + String paramsString = """ + "params":[ + {"_n1": [null, "8.15.0"]}, {"_n2": [null, null, "x"]}, {"_n3": [null, true, false]}, + {"_n4": [null, 1.0, null]}, {"_n5": [null, -799810013, null, 799810013]}, + {"n6" : [{"value" : {"a5" : "v5"}}]}, {"n7" : [{"identifier" : ["x", "y"]}]}, {"n8" : [{"pattern" : ["x*", "y*"]}]} + ]"""; + String json1 = String.format(Locale.ROOT, """ + { + %s, + "query": "%s", + "columnar": %s, + "locale": "%s", + "filter": %s + }""", paramsString, query, columnar, locale.toLanguageTag(), filter); + + Exception e1 = expectThrows(XContentParseException.class, () -> parseEsqlQueryRequestSync(json1)); + assertThat( + e1.getCause().getMessage(), + containsString("Failed to parse params: [3:2] _n1 parameter has values from different types, found NULL and KEYWORD") + ); + assertThat( + e1.getCause().getMessage(), + containsString("[3:29] _n2 parameter has values from different types, found NULL and KEYWORD") + ); + assertThat( + e1.getCause().getMessage(), + containsString("[3:57] _n3 parameter has values from different types, found NULL and BOOLEAN") + ); + assertThat( + e1.getCause().getMessage(), + containsString("[4:2] _n4 parameter has values from different types, found NULL and DOUBLE") + ); + assertThat( + e1.getCause().getMessage(), + containsString("[4:30] _n5 parameter has values from different types, found NULL and INTEGER") + ); + assertThat( + e1.getCause().getMessage(), + containsString("[5:2] n6=[{value={a5=v5}}] is not supported as a parameter") + ); + assertThat( + e1.getCause().getMessage(), + containsString("[5:40] n7=[{identifier=[x, y]}] is not supported as a parameter") + ); + assertThat( + e1.getCause().getMessage(), + containsString("[5:80] n8=[{pattern=[x*, y*]}] is not supported as a parameter") + ); + } + public void testInvalidParamsForIdentifiersPatterns() throws IOException { String query = randomAlphaOfLengthBetween(1, 100); boolean columnar = randomBoolean(); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java index 439c1bc189e3b..dc2bc7ef8b09e 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java @@ -2351,11 +2351,12 @@ public void testDenseVectorImplicitCastingKnn() { assumeTrue("dense_vector capability not available", EsqlCapabilities.Cap.KNN_FUNCTION_V5.isEnabled()); checkDenseVectorCastingKnn("float_vector"); + checkDenseVectorCastingKnn("byte_vector"); } private static void checkDenseVectorCastingKnn(String fieldName) { var plan = analyze(String.format(Locale.ROOT, """ - from test | where knn(%s, [0.342, 0.164, 0.234]) + from test | where knn(%s, [0, 1, 2]) """, fieldName), "mapping-dense_vector.json"); var limit = as(plan, Limit.class); @@ -2363,7 +2364,32 @@ private static void checkDenseVectorCastingKnn(String fieldName) { var knn = as(filter.condition(), Knn.class); var queryVector = as(knn.query(), Literal.class); assertEquals(DataType.DENSE_VECTOR, queryVector.dataType()); - assertThat(queryVector.value(), equalTo(List.of(0.342f, 0.164f, 0.234f))); + assertThat(queryVector.value(), equalTo(List.of(0f, 1f, 2f))); + } + + public void testDenseVectorImplicitCastingKnnQueryParams() { + assumeTrue("multi values for query params capability must be available", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled()); + + checkDenseVectorCastingKnnQueryParams("float_vector"); + checkDenseVectorCastingKnnQueryParams("byte_vector"); + } + + private void checkDenseVectorCastingKnnQueryParams(String fieldName) { + var plan = analyze(String.format(Locale.ROOT, """ + from test | where knn(%s, ?query_vector) + """, fieldName), "mapping-dense_vector.json", + new QueryParams( + List.of( + paramAsConstant("query_vector", List.of(0, 1, 2)) + ) + )); + + var limit = as(plan, Limit.class); + var filter = as(limit.child(), Filter.class); + var knn = as(filter.condition(), Knn.class); + var queryVector = as(knn.query(), Literal.class); + assertEquals(DataType.DENSE_VECTOR, queryVector.dataType()); + assertThat(queryVector.value(), equalTo(List.of(0f, 1f, 2f))); } public void testDenseVectorImplicitCastingSimilarityFunctions() { From a30d9aeda71430c23532c5ea6c08249b637def93 Mon Sep 17 00:00:00 2001 From: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com> Date: Mon, 8 Sep 2025 20:10:46 +0200 Subject: [PATCH 02/20] Update docs/changelog/134317.yaml --- docs/changelog/134317.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/134317.yaml diff --git a/docs/changelog/134317.yaml b/docs/changelog/134317.yaml new file mode 100644 index 0000000000000..972125d3b388e --- /dev/null +++ b/docs/changelog/134317.yaml @@ -0,0 +1,5 @@ +pr: 134317 +summary: ES|QL - Allow multivalued query parameters +area: "ES|QL, ES|QL" +type: enhancement +issues: [] From 3112666fe26d61887df8e63e3a17904b2b96d14b Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Mon, 8 Sep 2025 18:17:39 +0000 Subject: [PATCH 03/20] [CI] Auto commit changes from spotless --- .../xpack/esql/plugin/KnnFunctionIT.java | 10 +++++++--- .../xpack/esql/action/RequestXContent.java | 16 ++++++++-------- .../xpack/esql/action/EsqlQueryRequestTests.java | 15 +++------------ .../xpack/esql/analysis/AnalyzerTests.java | 14 ++++++-------- 4 files changed, 24 insertions(+), 31 deletions(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java index ed9e8f263e2f9..822d75bf38985 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java @@ -135,13 +135,17 @@ public void testKnnOptions() { } public void testDenseVectorQueryParams() { - assumeTrue("multi values for query params capability must be available", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled()); + assumeTrue( + "multi values for query params capability must be available", + EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled() + ); float[] queryVector = new float[numDims]; Arrays.fill(queryVector, 0); EsqlQueryRequest queryRequest = new EsqlQueryRequest(); - QueryParams queryParams = new QueryParams(List.of(new QueryParam("queryVector", Arrays.asList(queryVector), DataType.INTEGER, - ParserUtils.ParamClassification.PATTERN))); + QueryParams queryParams = new QueryParams( + List.of(new QueryParam("queryVector", Arrays.asList(queryVector), DataType.INTEGER, ParserUtils.ParamClassification.PATTERN)) + ); queryRequest.params(queryParams); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java index 375d52670314a..16d2a612f06fc 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java @@ -189,20 +189,20 @@ private static QueryParams parseParams(XContentParser p) throws IOException { checkParamValueValidity(classification, currentValue, loc, errors); DataType currentType = DataType.fromJava(currentValue); if (currentType == null) { + errors.add(new XContentParseException(loc, entry + " is not supported as a parameter")); + break; + } else if ((type != null) && (type != currentType)) { errors.add( new XContentParseException( loc, - entry + " is not supported as a parameter" + paramName + + " parameter has values from different types, found " + + type + + " and " + + currentType ) ); break; - } else if ((type != null) && (type != currentType)) { - errors.add(new XContentParseException( - loc, - paramName + " parameter has values from different types, found " + type + " and " - + currentType - )); - break; } type = currentType; values.add(currentValue); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index 7d520a4f465f2..438748e5cdb7e 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -354,18 +354,9 @@ public void testInvalidMultivaluedParams() throws IOException { e1.getCause().getMessage(), containsString("[4:30] _n5 parameter has values from different types, found NULL and INTEGER") ); - assertThat( - e1.getCause().getMessage(), - containsString("[5:2] n6=[{value={a5=v5}}] is not supported as a parameter") - ); - assertThat( - e1.getCause().getMessage(), - containsString("[5:40] n7=[{identifier=[x, y]}] is not supported as a parameter") - ); - assertThat( - e1.getCause().getMessage(), - containsString("[5:80] n8=[{pattern=[x*, y*]}] is not supported as a parameter") - ); + assertThat(e1.getCause().getMessage(), containsString("[5:2] n6=[{value={a5=v5}}] is not supported as a parameter")); + assertThat(e1.getCause().getMessage(), containsString("[5:40] n7=[{identifier=[x, y]}] is not supported as a parameter")); + assertThat(e1.getCause().getMessage(), containsString("[5:80] n8=[{pattern=[x*, y*]}] is not supported as a parameter")); } public void testInvalidParamsForIdentifiersPatterns() throws IOException { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java index dc2bc7ef8b09e..ac18865ec1f7d 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java @@ -2368,7 +2368,10 @@ private static void checkDenseVectorCastingKnn(String fieldName) { } public void testDenseVectorImplicitCastingKnnQueryParams() { - assumeTrue("multi values for query params capability must be available", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled()); + assumeTrue( + "multi values for query params capability must be available", + EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled() + ); checkDenseVectorCastingKnnQueryParams("float_vector"); checkDenseVectorCastingKnnQueryParams("byte_vector"); @@ -2376,13 +2379,8 @@ public void testDenseVectorImplicitCastingKnnQueryParams() { private void checkDenseVectorCastingKnnQueryParams(String fieldName) { var plan = analyze(String.format(Locale.ROOT, """ - from test | where knn(%s, ?query_vector) - """, fieldName), "mapping-dense_vector.json", - new QueryParams( - List.of( - paramAsConstant("query_vector", List.of(0, 1, 2)) - ) - )); + from test | where knn(%s, ?query_vector) + """, fieldName), "mapping-dense_vector.json", new QueryParams(List.of(paramAsConstant("query_vector", List.of(0, 1, 2))))); var limit = as(plan, Limit.class); var filter = as(limit.child(), Filter.class); From f4f06788a4afb8a002c0c7c96248416631e1fde0 Mon Sep 17 00:00:00 2001 From: cdelgado Date: Tue, 9 Sep 2025 10:24:39 +0200 Subject: [PATCH 04/20] Fix changelog --- docs/changelog/134317.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/changelog/134317.yaml b/docs/changelog/134317.yaml index 972125d3b388e..c06fa0709cfaa 100644 --- a/docs/changelog/134317.yaml +++ b/docs/changelog/134317.yaml @@ -1,5 +1,5 @@ pr: 134317 summary: ES|QL - Allow multivalued query parameters -area: "ES|QL, ES|QL" +area: "ES|QL" type: enhancement -issues: [] +issues: [127556] From cc4d17ecf7db14c27d83187fdc6499d076096655 Mon Sep 17 00:00:00 2001 From: cdelgado Date: Tue, 9 Sep 2025 11:42:57 +0200 Subject: [PATCH 05/20] Fix param validation --- .../xpack/esql/action/RequestXContent.java | 97 ++++++++++--------- .../esql/action/EsqlQueryRequestTests.java | 67 ++++++------- 2 files changed, 80 insertions(+), 84 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java index 16d2a612f06fc..9e55ece7a37fe 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java @@ -176,58 +176,22 @@ private static QueryParams parseParams(XContentParser p) throws IOException { classification = getParamClassification(keyName.toString(), errors, loc); if (classification != null) { paramValue = value.get(keyName); - checkParamValueValidity(classification, paramValue, loc, errors); + checkParamValueValidity(entry, classification, paramValue, loc, errors); } } - } else {// parameter specifies as a value only + } else {// parameter specifies a single or multi value paramValue = entry.getValue(); classification = VALUE; - if (paramValue instanceof List) { - type = null; - List values = new ArrayList<>(); - for (Object currentValue : (List) paramValue) { - checkParamValueValidity(classification, currentValue, loc, errors); - DataType currentType = DataType.fromJava(currentValue); - if (currentType == null) { - errors.add(new XContentParseException(loc, entry + " is not supported as a parameter")); - break; - } else if ((type != null) && (type != currentType)) { - errors.add( - new XContentParseException( - loc, - paramName - + " parameter has values from different types, found " - + type - + " and " - + currentType - ) - ); - break; - } - type = currentType; - values.add(currentValue); - } - currentParam = new QueryParam( - paramName, - values, - (classification == VALUE) ? type : DataType.NULL, - classification - ); - namedParams.add(currentParam); - } else { - type = DataType.fromJava(paramValue); - if (type == null) { - errors.add(new XContentParseException(loc, entry + " is not supported as a parameter")); - } - currentParam = new QueryParam( - paramName, - paramValue, - (classification == VALUE) ? type : DataType.NULL, - classification - ); - namedParams.add(currentParam); - } + checkParamValueValidity(entry, classification, paramValue, loc, errors); } + type = DataType.fromJava(paramValue); + currentParam = new QueryParam( + paramName, + paramValue, + (classification == VALUE) ? type : DataType.NULL, + classification + ); + namedParams.add(currentParam); } } else { paramValue = null; @@ -350,11 +314,50 @@ private static ParserUtils.ParamClassification getParamClassification( } private static void checkParamValueValidity( + Map.Entry entry, ParserUtils.ParamClassification classification, Object value, XContentLocation loc, List errors ) { + if (value instanceof List valueList) { + if (classification != VALUE) { + errors.add( + new XContentParseException( + loc, + entry + " parameter is multivalued, only " + VALUE.name() + " parameters can be multivalued" + ) + ); + return; + } + // Multivalued field + DataType type = null; + for (Object currentValue : valueList) { + checkParamValueValidity(entry, classification, currentValue, loc, errors); + DataType currentType = DataType.fromJava(currentValue); + if ((type != null) && (type != currentType)) { + errors.add( + new XContentParseException( + loc, + entry.getKey() + + " parameter has values from different types, found " + + type + + " and " + + currentType + ) + ); + break; + } + type = currentType; + } + return; + } + + DataType type = DataType.fromJava(value); + if (type == null) { + errors.add(new XContentParseException(loc, entry + " is not supported as a parameter")); + } + // If a param is an "identifier" or a "pattern", validate it is a string. // If a param is a "pattern", validate it contains *. if (classification == IDENTIFIER || classification == PATTERN) { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index 438748e5cdb7e..8d3183a2dca05 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -166,7 +166,7 @@ public void testNamedMultivaluedParams() throws IOException { String paramsString = """ ,"params":[ {"_n1": ["8.15.0", "8.19.0"]}, {"_n2": ["x", "y"]}, {"_n3": [true, false]}, {"_n4": [1.0, 1.1, 1.2]}, - {"_n5": [-799810013, 0, 799810013]}, {"_n6": [null, null, null]}, + {"_n5": [-799810013, 0, 799810013]}, {"_n6": [null, null, null]} ] }"""; List params = List.of( @@ -383,42 +383,35 @@ public void testInvalidParamsForIdentifiersPatterns() throws IOException { }""", paramsString1, query, columnar, locale.toLanguageTag(), filter); Exception e1 = expectThrows(XContentParseException.class, () -> parseEsqlQueryRequestSync(json1)); - assertThat( - e1.getCause().getMessage(), - containsString( - "[2:15] [v] is not a valid param attribute, a valid attribute is any of VALUE, IDENTIFIER, PATTERN; " - + "[2:38] [n2] has multiple param attributes [identifier, pattern], " - + "only one of VALUE, IDENTIFIER, PATTERN can be defined in a param; " - + "[2:38] [v2] is not a valid value for PATTERN parameter, " - + "a valid value for PATTERN parameter is a string and contains *; " - + "[3:1] [n3] has multiple param attributes [identifier, pattern], " - + "only one of VALUE, IDENTIFIER, PATTERN can be defined in a param; " - + "[3:1] [v3] is not a valid value for PATTERN parameter, " - + "a valid value for PATTERN parameter is a string and contains *; " - + "[3:51] [n4] has multiple param attributes [pattern, value], " - + "only one of VALUE, IDENTIFIER, PATTERN can be defined in a param; " - + "[3:51] [v4.1] is not a valid value for PATTERN parameter, " - + "a valid value for PATTERN parameter is a string and contains *; " - + "[4:1] n5={value={a5=v5}} is not supported as a parameter; " - + "[4:36] [{a6.1=v6.1, a6.2=v6.2}] is not a valid value for IDENTIFIER parameter, " - + "a valid value for IDENTIFIER parameter is a string; " - + "[4:36] n6={identifier={a6.1=v6.1, a6.2=v6.2}} is not supported as a parameter; " - + "[4:98] [n7] has no valid param attribute, only one of VALUE, IDENTIFIER, PATTERN can be defined in a param; " - + "[5:1] n8={value=[x, y]} is not supported as a parameter; " - + "[5:34] [[x, y]] is not a valid value for IDENTIFIER parameter, a valid value for IDENTIFIER parameter is a string; " - + "[5:34] n9={identifier=[x, y]} is not supported as a parameter; " - + "[5:72] [[x*, y*]] is not a valid value for PATTERN parameter, " - + "a valid value for PATTERN parameter is a string and contains *; " - + "[5:72] n10={pattern=[x*, y*]} is not supported as a parameter; " - + "[6:1] [1] is not a valid value for IDENTIFIER parameter, a valid value for IDENTIFIER parameter is a string; " - + "[6:31] [true] is not a valid value for PATTERN parameter, " - + "a valid value for PATTERN parameter is a string and contains *; " - + "[6:61] [null] is not a valid value for IDENTIFIER parameter, a valid value for IDENTIFIER parameter is a string; " - + "[6:94] [v14] is not a valid value for PATTERN parameter, " - + "a valid value for PATTERN parameter is a string and contains *; " - + "[7:1] Cannot parse more than one key:value pair as parameter, found [{n16:{identifier=v16}}, {n15:{pattern=v15*}}]" - ) - ); + String message = e1.getCause().getMessage(); + assertThat(message, containsString("[2:15] [v] is not a valid param attribute, a valid attribute is any of VALUE, IDENTIFIER, PATTERN; ")); + assertThat(message, containsString("[2:38] [n2] has multiple param attributes [identifier, pattern],")); + assertThat(message, containsString("only one of VALUE, IDENTIFIER, PATTERN can be defined in a param;")); + assertThat(message, containsString("[2:38] [v2] is not a valid value for PATTERN parameter,")); + assertThat(message, containsString("a valid value for PATTERN parameter is a string and contains *;")); + assertThat(message, containsString("[3:1] [n3] has multiple param attributes [identifier, pattern],")); + assertThat(message, containsString("only one of VALUE, IDENTIFIER, PATTERN can be defined in a param;")); + assertThat(message, containsString("[3:1] [v3] is not a valid value for PATTERN parameter,")); + assertThat(message, containsString("a valid value for PATTERN parameter is a string and contains *;")); + assertThat(message, containsString("[3:51] [n4] has multiple param attributes [pattern, value],")); + assertThat(message, containsString("only one of VALUE, IDENTIFIER, PATTERN can be defined in a param;")); + assertThat(message, containsString("[3:51] [v4.1] is not a valid value for PATTERN parameter,")); + assertThat(message, containsString("a valid value for PATTERN parameter is a string and contains *;")); + assertThat(message, containsString("[4:1] n5={value={a5=v5}} is not supported as a parameter;")); + assertThat(message, containsString("[4:36] [{a6.1=v6.1, a6.2=v6.2}] is not a valid value for IDENTIFIER parameter,")); + assertThat(message, containsString("a valid value for IDENTIFIER parameter is a string;")); + assertThat(message, containsString("[4:36] n6={identifier={a6.1=v6.1, a6.2=v6.2}} is not supported as a parameter;")); + assertThat(message, containsString("[4:98] [n7] has no valid param attribute, only one of VALUE, IDENTIFIER, PATTERN can be defined in a param;")); + assertThat(message, containsString("[5:34] n9={identifier=[x, y]} parameter is multivalued, only VALUE parameters can be multivalued;")); + assertThat(message, containsString("[5:72] n10={pattern=[x*, y*]} parameter is multivalued, only VALUE parameters can be multivalued;")); + assertThat(message, containsString("a valid value for PATTERN parameter is a string and contains *;")); + assertThat(message, containsString("[6:1] [1] is not a valid value for IDENTIFIER parameter, a valid value for IDENTIFIER parameter is a string;")); + assertThat(message, containsString("[6:31] [true] is not a valid value for PATTERN parameter,")); + assertThat(message, containsString("a valid value for PATTERN parameter is a string and contains *;")); + assertThat(message, containsString("[6:61] [null] is not a valid value for IDENTIFIER parameter, a valid value for IDENTIFIER parameter is a string;")); + assertThat(message, containsString("[6:94] [v14] is not a valid value for PATTERN parameter,")); + assertThat(message, containsString("a valid value for PATTERN parameter is a string and contains *;")); + assertThat(message, containsString("[7:1] Cannot parse more than one key:value pair as parameter, found [{n16:{identifier=v16}}, {n15:{pattern=v15*}}")); } // Test for https://github.com/elastic/elasticsearch/issues/110028 From a72a293ea38e1fbf06e70f62075ccd1eeb162cce Mon Sep 17 00:00:00 2001 From: cdelgado Date: Tue, 9 Sep 2025 12:38:35 +0200 Subject: [PATCH 06/20] Add REST tests and fix parsing --- .../xpack/esql/qa/rest/RestEsqlTestCase.java | 37 +++++++++++++++++-- .../xpack/esql/parser/ExpressionBuilder.java | 8 +++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java index 61cc5c80cbc00..d9c5e697779f9 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java @@ -687,14 +687,45 @@ public void testErrorMessageForInvalidIntervalParams() throws IOException { ); } - public void testErrorMessageForArrayValuesInParams() throws IOException { + public void testArrayValuesAllowedInValueParams() throws IOException { + Map responseMap = runEsql( + RequestObjectBuilder.jsonBuilder() + .query("row a = ?n1 | eval s = ?n2") + .params("[{\"n1\" : [\"a1\", \"a2\"]}, {\"n2\" : [1, 2]}]") + ); + System.out.println(responseMap); + + ListMatcher values = matchesList().item( + matchesList().item(matchesList().item("a1").item("a2")).item(matchesList().item(1).item(2)) + ); + + assertResultMap( + responseMap, + matchesList().item(matchesMap().entry("name", "a").entry("type", "keyword")) + .item(matchesMap().entry("name", "s").entry("type", "integer")), + values + ); + } + + public void testErrorMessageForArrayValuesInNonValueParams() throws IOException { ResponseException re = expectThrows( ResponseException.class, - () -> runEsql(RequestObjectBuilder.jsonBuilder().query("row a = 1 | eval x = ?").params("[{\"n1\": [5, 6, 7]}]")) + () -> runEsql( + RequestObjectBuilder.jsonBuilder().query("row a = 1 | eval ?n1 = 5").params("[{\"n1\" : {\"identifier\" : [\"integer\"]}}]") + ) + ); + assertThat( + EntityUtils.toString(re.getResponse().getEntity()), + containsString("\"Failed to parse params: [1:47] n1={identifier=[integer]} parameter is multivalued") + ); + + re = expectThrows( + ResponseException.class, + () -> runEsql(RequestObjectBuilder.jsonBuilder().query("row a = 1 | keep ?n1").params("[{\"n1\" : {\"pattern\" : [\"a*\"]}}]")) ); assertThat( EntityUtils.toString(re.getResponse().getEntity()), - containsString("Failed to parse params: [1:45] n1=[5, 6, 7] is not supported as a parameter") + containsString("\"Failed to parse params: [1:43] n1={pattern=[a*]} parameter is multivalued") ); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/ExpressionBuilder.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/ExpressionBuilder.java index 451be4db743da..e726b48888a73 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/ExpressionBuilder.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/ExpressionBuilder.java @@ -1047,8 +1047,12 @@ private Expression visitParam(EsqlBaseParser.ParameterContext ctx, QueryParam pa return new UnresolvedAttribute(source, value.toString()); } } - if ((type == KEYWORD || type == TEXT) && value instanceof String) { - value = BytesRefs.toBytesRef(value); + if ((type == KEYWORD || type == TEXT)) { + if (value instanceof String) { + value = BytesRefs.toBytesRef(value); + } else if (value instanceof List list) { + value = list.stream().map(v -> v instanceof String ? BytesRefs.toBytesRef(v) : v).toList(); + } } return new Literal(source, value, type); } From 1dd6669d5b4f6044613f377e6a6b94085f34de8a Mon Sep 17 00:00:00 2001 From: cdelgado Date: Tue, 9 Sep 2025 12:38:38 +0200 Subject: [PATCH 07/20] Spotless --- .../xpack/esql/action/RequestXContent.java | 6 +-- .../esql/action/EsqlQueryRequestTests.java | 39 +++++++++++++++---- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java index 9e55ece7a37fe..2038a38791dbc 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java @@ -339,11 +339,7 @@ private static void checkParamValueValidity( errors.add( new XContentParseException( loc, - entry.getKey() - + " parameter has values from different types, found " - + type - + " and " - + currentType + entry.getKey() + " parameter has values from different types, found " + type + " and " + currentType ) ); break; diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index 8d3183a2dca05..94b4bd0d4fd8a 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -384,7 +384,10 @@ public void testInvalidParamsForIdentifiersPatterns() throws IOException { Exception e1 = expectThrows(XContentParseException.class, () -> parseEsqlQueryRequestSync(json1)); String message = e1.getCause().getMessage(); - assertThat(message, containsString("[2:15] [v] is not a valid param attribute, a valid attribute is any of VALUE, IDENTIFIER, PATTERN; ")); + assertThat( + message, + containsString("[2:15] [v] is not a valid param attribute, a valid attribute is any of VALUE, IDENTIFIER, PATTERN; ") + ); assertThat(message, containsString("[2:38] [n2] has multiple param attributes [identifier, pattern],")); assertThat(message, containsString("only one of VALUE, IDENTIFIER, PATTERN can be defined in a param;")); assertThat(message, containsString("[2:38] [v2] is not a valid value for PATTERN parameter,")); @@ -401,17 +404,39 @@ public void testInvalidParamsForIdentifiersPatterns() throws IOException { assertThat(message, containsString("[4:36] [{a6.1=v6.1, a6.2=v6.2}] is not a valid value for IDENTIFIER parameter,")); assertThat(message, containsString("a valid value for IDENTIFIER parameter is a string;")); assertThat(message, containsString("[4:36] n6={identifier={a6.1=v6.1, a6.2=v6.2}} is not supported as a parameter;")); - assertThat(message, containsString("[4:98] [n7] has no valid param attribute, only one of VALUE, IDENTIFIER, PATTERN can be defined in a param;")); - assertThat(message, containsString("[5:34] n9={identifier=[x, y]} parameter is multivalued, only VALUE parameters can be multivalued;")); - assertThat(message, containsString("[5:72] n10={pattern=[x*, y*]} parameter is multivalued, only VALUE parameters can be multivalued;")); + assertThat( + message, + containsString("[4:98] [n7] has no valid param attribute, only one of VALUE, IDENTIFIER, PATTERN can be defined in a param;") + ); + assertThat( + message, + containsString("[5:34] n9={identifier=[x, y]} parameter is multivalued, only VALUE parameters can be multivalued;") + ); + assertThat( + message, + containsString("[5:72] n10={pattern=[x*, y*]} parameter is multivalued, only VALUE parameters can be multivalued;") + ); assertThat(message, containsString("a valid value for PATTERN parameter is a string and contains *;")); - assertThat(message, containsString("[6:1] [1] is not a valid value for IDENTIFIER parameter, a valid value for IDENTIFIER parameter is a string;")); + assertThat( + message, + containsString("[6:1] [1] is not a valid value for IDENTIFIER parameter, a valid value for IDENTIFIER parameter is a string;") + ); assertThat(message, containsString("[6:31] [true] is not a valid value for PATTERN parameter,")); assertThat(message, containsString("a valid value for PATTERN parameter is a string and contains *;")); - assertThat(message, containsString("[6:61] [null] is not a valid value for IDENTIFIER parameter, a valid value for IDENTIFIER parameter is a string;")); + assertThat( + message, + containsString( + "[6:61] [null] is not a valid value for IDENTIFIER parameter, a valid value for IDENTIFIER parameter is a string;" + ) + ); assertThat(message, containsString("[6:94] [v14] is not a valid value for PATTERN parameter,")); assertThat(message, containsString("a valid value for PATTERN parameter is a string and contains *;")); - assertThat(message, containsString("[7:1] Cannot parse more than one key:value pair as parameter, found [{n16:{identifier=v16}}, {n15:{pattern=v15*}}")); + assertThat( + message, + containsString( + "[7:1] Cannot parse more than one key:value pair as parameter, found [{n16:{identifier=v16}}, {n15:{pattern=v15*}}" + ) + ); } // Test for https://github.com/elastic/elasticsearch/issues/110028 From 2c95a66761f285d9995e9b7d10f737193b7c8454 Mon Sep 17 00:00:00 2001 From: cdelgado Date: Tue, 9 Sep 2025 12:41:32 +0200 Subject: [PATCH 08/20] Add capabilities guard --- .../elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java index d9c5e697779f9..ed2e7be8393cf 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java @@ -688,6 +688,8 @@ public void testErrorMessageForInvalidIntervalParams() throws IOException { } public void testArrayValuesAllowedInValueParams() throws IOException { + assumeTrue("multivalues for params", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled()); + Map responseMap = runEsql( RequestObjectBuilder.jsonBuilder() .query("row a = ?n1 | eval s = ?n2") @@ -708,6 +710,8 @@ public void testArrayValuesAllowedInValueParams() throws IOException { } public void testErrorMessageForArrayValuesInNonValueParams() throws IOException { + assumeTrue("multivalues for params", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled()); + ResponseException re = expectThrows( ResponseException.class, () -> runEsql( From ceccf94e9cc4fa785f715808ad9a4b62c457faca Mon Sep 17 00:00:00 2001 From: cdelgado Date: Tue, 9 Sep 2025 16:52:27 +0200 Subject: [PATCH 09/20] Handle null multivalues --- .../xpack/esql/qa/rest/RestEsqlTestCase.java | 51 +++++++++++- .../xpack/esql/action/RequestXContent.java | 78 +++++++++++++------ .../xpack/esql/evaluator/EvalMapper.java | 13 +++- 3 files changed, 114 insertions(+), 28 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java index ed2e7be8393cf..874f49d34e348 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java @@ -692,19 +692,62 @@ public void testArrayValuesAllowedInValueParams() throws IOException { Map responseMap = runEsql( RequestObjectBuilder.jsonBuilder() - .query("row a = ?n1 | eval s = ?n2") - .params("[{\"n1\" : [\"a1\", \"a2\"]}, {\"n2\" : [1, 2]}]") + .query("row a = ?a | eval b = ?b, c = ?c, d = ?d, e = ?e, f = ?f") + .params( + "[{\"a\" : [\"a1\", \"a2\"]}, {\"b\" : [1, 2]}, {\"c\": [true, false]}, {\"d\": [1.1, 2.2]}," + + " {\"e\": [1674835275193, 1674835275193]}, {\"f\": [null, null]}]" + ) + ); + System.out.println(responseMap); + + ListMatcher values = matchesList().item( + matchesList().item(matchesList().item("a1").item("a2")) + .item(matchesList().item(1).item(2)) + .item(matchesList().item(true).item(false)) + .item(matchesList().item(1.1).item(2.2)) + .item(matchesList().item(1674835275193L).item(1674835275193L)) + .item(null) // constant null block, no multi-value + ); + + assertResultMap( + responseMap, + matchesList().item(matchesMap().entry("name", "a").entry("type", "keyword")) + .item(matchesMap().entry("name", "b").entry("type", "integer")) + .item(matchesMap().entry("name", "c").entry("type", "boolean")) + .item(matchesMap().entry("name", "d").entry("type", "double")) + .item(matchesMap().entry("name", "e").entry("type", "long")) + .item(matchesMap().entry("name", "f").entry("type", "null")), + values + ); + } + + public void testArrayValuesAllowedInUnnamedParams() throws IOException { + assumeTrue("multivalues for params", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled()); + + Map responseMap = runEsql( + RequestObjectBuilder.jsonBuilder() + .query("row a = ? | eval b = ?, c = ?, d = ?, e = ?, f = ?") + .params("[[\"a1\", \"a2\"], [1, 2], [true, false], [1.1, 2.2], [1674835275193, 1674835275193], [null, null]]") ); System.out.println(responseMap); ListMatcher values = matchesList().item( - matchesList().item(matchesList().item("a1").item("a2")).item(matchesList().item(1).item(2)) + matchesList().item(matchesList().item("a1").item("a2")) + .item(matchesList().item(1).item(2)) + .item(matchesList().item(true).item(false)) + .item(matchesList().item(1.1).item(2.2)) + .item(matchesList().item(1674835275193L).item(1674835275193L)) + .item(null) ); assertResultMap( responseMap, matchesList().item(matchesMap().entry("name", "a").entry("type", "keyword")) - .item(matchesMap().entry("name", "s").entry("type", "integer")), + .item(matchesMap().entry("name", "b").entry("type", "integer")) + .item(matchesMap().entry("name", "c").entry("type", "boolean")) + .item(matchesMap().entry("name", "d").entry("type", "double")) + .item(matchesMap().entry("name", "e").entry("type", "long")) + .item(matchesMap().entry("name", "f").entry("type", "null")), values ); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java index 2038a38791dbc..c48f5c7be452d 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java @@ -194,32 +194,31 @@ private static QueryParams parseParams(XContentParser p) throws IOException { namedParams.add(currentParam); } } else { - paramValue = null; - if (token == XContentParser.Token.VALUE_STRING) { - paramValue = p.text(); - type = DataType.KEYWORD; - } else if (token == XContentParser.Token.VALUE_NUMBER) { - XContentParser.NumberType numberType = p.numberType(); - if (numberType == XContentParser.NumberType.INT) { - paramValue = p.intValue(); - type = DataType.INTEGER; - } else if (numberType == XContentParser.NumberType.LONG) { - paramValue = p.longValue(); - type = DataType.LONG; - } else if (numberType == XContentParser.NumberType.DOUBLE) { - paramValue = p.doubleValue(); - type = DataType.DOUBLE; + if (token == XContentParser.Token.START_ARRAY) { + DataType currentType = null; + List paramValues = new ArrayList<>(); + while ((p.nextToken()) != XContentParser.Token.END_ARRAY) { + ParamValueAndType valueAndDataType = parseSingleParamValue(p, errors); + if (currentType == null) { + currentType = valueAndDataType.type; + } else if (currentType != valueAndDataType.type) { + errors.add( + new XContentParseException( + loc, + "Unnamed parameter has values from different types, found " + + currentType + + " and " + + valueAndDataType.type + ) + ); + } + paramValues.add(valueAndDataType.value); } - } else if (token == XContentParser.Token.VALUE_BOOLEAN) { - paramValue = p.booleanValue(); - type = DataType.BOOLEAN; - } else if (token == XContentParser.Token.VALUE_NULL) { - type = DataType.NULL; + unNamedParams.add(new QueryParam(null, paramValues, currentType, VALUE)); } else { - errors.add(new XContentParseException(loc, token + " is not supported as a parameter")); + ParamValueAndType valueAndDataType = parseSingleParamValue(p, errors); + unNamedParams.add(new QueryParam(null, valueAndDataType.value, valueAndDataType.type, VALUE)); } - currentParam = new QueryParam(null, paramValue, type, VALUE); - unNamedParams.add(currentParam); } } } @@ -243,6 +242,39 @@ private static QueryParams parseParams(XContentParser p) throws IOException { return new QueryParams(namedParams.isEmpty() ? unNamedParams : namedParams); } + private record ParamValueAndType(Object value, DataType type) {} + + private static ParamValueAndType parseSingleParamValue(XContentParser p, List errors) throws IOException { + Object paramValue = null; + DataType type = null; + XContentParser.Token token = p.currentToken(); + if (token == XContentParser.Token.VALUE_STRING) { + paramValue = p.text(); + type = DataType.KEYWORD; + } else if (token == XContentParser.Token.VALUE_NUMBER) { + XContentParser.NumberType numberType = p.numberType(); + if (numberType == XContentParser.NumberType.INT) { + paramValue = p.intValue(); + type = DataType.INTEGER; + } else if (numberType == XContentParser.NumberType.LONG) { + paramValue = p.longValue(); + type = DataType.LONG; + } else if (numberType == XContentParser.NumberType.DOUBLE) { + paramValue = p.doubleValue(); + type = DataType.DOUBLE; + } + } else if (token == XContentParser.Token.VALUE_BOOLEAN) { + paramValue = p.booleanValue(); + type = DataType.BOOLEAN; + } else if (token == XContentParser.Token.VALUE_NULL) { + type = DataType.NULL; + } else { + XContentLocation loc = p.getTokenLocation(); + errors.add(new XContentParseException(loc, token + " is not supported as a parameter")); + } + return new ParamValueAndType(paramValue, type); + } + private static void checkParamNameValidity(String name, List errors, XContentLocation loc) { if (isValidParamName(name) == false) { errors.add( diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/EvalMapper.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/EvalMapper.java index 81f58cd2f24a0..949e7d8b7f250 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/EvalMapper.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/EvalMapper.java @@ -273,7 +273,18 @@ private static Block block(Literal lit, BlockFactory blockFactory, int positions if (multiValue.isEmpty()) { return blockFactory.newConstantNullBlock(positions); } - var wrapper = BlockUtils.wrapperFor(blockFactory, ElementType.fromJava(multiValue.get(0).getClass()), positions); + ElementType type = ElementType.NULL; + for (Object elem : multiValue) { + if (elem != null) { + type = ElementType.fromJava(elem.getClass()); + break; + } + } + if (type == ElementType.NULL) { + // all values are null + return blockFactory.newConstantNullBlock(positions); + } + var wrapper = BlockUtils.wrapperFor(blockFactory, type, positions); for (int i = 0; i < positions; i++) { wrapper.accept(multiValue); } From ab1146d10e923acfb938b92cb82a60e84676de80 Mon Sep 17 00:00:00 2001 From: cdelgado Date: Wed, 10 Sep 2025 08:16:02 +0200 Subject: [PATCH 10/20] Don't allow null in multivalues --- .../xpack/esql/qa/rest/RestEsqlTestCase.java | 44 ++++++++----- .../xpack/esql/action/RequestXContent.java | 39 ++++++----- .../rest-api-spec/test/esql/10_basic.yml | 65 +++++++++++++++++++ 3 files changed, 116 insertions(+), 32 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java index 874f49d34e348..5ffa5c913833c 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java @@ -692,10 +692,10 @@ public void testArrayValuesAllowedInValueParams() throws IOException { Map responseMap = runEsql( RequestObjectBuilder.jsonBuilder() - .query("row a = ?a | eval b = ?b, c = ?c, d = ?d, e = ?e, f = ?f") + .query("row a = ?a | eval b = ?b, c = ?c, d = ?d, e = ?e") .params( "[{\"a\" : [\"a1\", \"a2\"]}, {\"b\" : [1, 2]}, {\"c\": [true, false]}, {\"d\": [1.1, 2.2]}," - + " {\"e\": [1674835275193, 1674835275193]}, {\"f\": [null, null]}]" + + " {\"e\": [1674835275193, 1674835275193]}]" ) ); System.out.println(responseMap); @@ -706,19 +706,31 @@ public void testArrayValuesAllowedInValueParams() throws IOException { .item(matchesList().item(true).item(false)) .item(matchesList().item(1.1).item(2.2)) .item(matchesList().item(1674835275193L).item(1674835275193L)) - .item(null) // constant null block, no multi-value ); + } - assertResultMap( - responseMap, - matchesList().item(matchesMap().entry("name", "a").entry("type", "keyword")) - .item(matchesMap().entry("name", "b").entry("type", "integer")) - .item(matchesMap().entry("name", "c").entry("type", "boolean")) - .item(matchesMap().entry("name", "d").entry("type", "double")) - .item(matchesMap().entry("name", "e").entry("type", "long")) - .item(matchesMap().entry("name", "f").entry("type", "null")), - values + public void testArrayValuesNullsNotAllowedInValueParams() throws IOException { + assumeTrue("multivalues for params", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled()); + + ResponseException re = expectThrows( + ResponseException.class, + () -> runEsqlSync( + RequestObjectBuilder.jsonBuilder() + .query("row a = ?a | eval b = ?b, c = ?c, d = ?d, e = ?e, f = ?f") + .params( + "[{\"a\" : [null, \"a2\"]}, {\"b\" : [null, 2]}, {\"c\": [null, false]}, {\"d\": [null, 2.2]}," + + " {\"e\": [null, 1674835275193]}, {\"f\": [null, null]}]" + ) + ) ); + + String error = EntityUtils.toString(re.getResponse().getEntity()).replaceAll("\\\\\n\s+\\\\", ""); + assertThat(error, containsString("[1:79] Parameter [a] contains a null value. Null values are not allowed for multivalues;")); + assertThat(error, containsString("[1:101] Parameter [b] contains a null value. Null values are not allowed for multivalues;")); + assertThat(error, containsString("[1:120] Parameter [c] contains a null value. Null values are not allowed for multivalues;")); + assertThat(error, containsString("[1:142] Parameter [d] contains a null value. Null values are not allowed for multivalues;")); + assertThat(error, containsString("[1:162] Parameter [e] contains a null value. Null values are not allowed for multivalues;")); + assertThat(error, containsString("[1:192] Parameter [f] contains a null value. Null values are not allowed for multivalues;")); } public void testArrayValuesAllowedInUnnamedParams() throws IOException { @@ -726,8 +738,8 @@ public void testArrayValuesAllowedInUnnamedParams() throws IOException { Map responseMap = runEsql( RequestObjectBuilder.jsonBuilder() - .query("row a = ? | eval b = ?, c = ?, d = ?, e = ?, f = ?") - .params("[[\"a1\", \"a2\"], [1, 2], [true, false], [1.1, 2.2], [1674835275193, 1674835275193], [null, null]]") + .query("row a = ? | eval b = ?, c = ?, d = ?, e = ?") + .params("[[\"a1\", \"a2\"], [1, 2], [true, false], [1.1, 2.2], [1674835275193, 1674835275193]]") ); System.out.println(responseMap); @@ -737,7 +749,6 @@ public void testArrayValuesAllowedInUnnamedParams() throws IOException { .item(matchesList().item(true).item(false)) .item(matchesList().item(1.1).item(2.2)) .item(matchesList().item(1674835275193L).item(1674835275193L)) - .item(null) ); assertResultMap( @@ -746,8 +757,7 @@ public void testArrayValuesAllowedInUnnamedParams() throws IOException { .item(matchesMap().entry("name", "b").entry("type", "integer")) .item(matchesMap().entry("name", "c").entry("type", "boolean")) .item(matchesMap().entry("name", "d").entry("type", "double")) - .item(matchesMap().entry("name", "e").entry("type", "long")) - .item(matchesMap().entry("name", "f").entry("type", "null")), + .item(matchesMap().entry("name", "e").entry("type", "long")), values ); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java index c48f5c7be452d..f72ab0887600e 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java @@ -195,26 +195,27 @@ private static QueryParams parseParams(XContentParser p) throws IOException { } } else { if (token == XContentParser.Token.START_ARRAY) { - DataType currentType = null; + DataType arrayType = null; List paramValues = new ArrayList<>(); while ((p.nextToken()) != XContentParser.Token.END_ARRAY) { ParamValueAndType valueAndDataType = parseSingleParamValue(p, errors); - if (currentType == null) { - currentType = valueAndDataType.type; - } else if (currentType != valueAndDataType.type) { + DataType currentType = valueAndDataType.type; + if (currentType == DataType.NULL) { + errors.add(new XContentParseException(loc, "Unnamed parameter contains a null in a multivalued value")); + continue; + } + if (arrayType != null && arrayType != currentType) { errors.add( new XContentParseException( loc, - "Unnamed parameter has values from different types, found " - + currentType - + " and " - + valueAndDataType.type + "Unnamed parameter has values from different types, found " + arrayType + " and " + currentType ) ); } + arrayType = currentType; paramValues.add(valueAndDataType.value); } - unNamedParams.add(new QueryParam(null, paramValues, currentType, VALUE)); + unNamedParams.add(new QueryParam(null, paramValues, arrayType, VALUE)); } else { ParamValueAndType valueAndDataType = parseSingleParamValue(p, errors); unNamedParams.add(new QueryParam(null, valueAndDataType.value, valueAndDataType.type, VALUE)); @@ -246,7 +247,7 @@ private record ParamValueAndType(Object value, DataType type) {} private static ParamValueAndType parseSingleParamValue(XContentParser p, List errors) throws IOException { Object paramValue = null; - DataType type = null; + DataType type = DataType.NULL; XContentParser.Token token = p.currentToken(); if (token == XContentParser.Token.VALUE_STRING) { paramValue = p.text(); @@ -363,20 +364,28 @@ private static void checkParamValueValidity( return; } // Multivalued field - DataType type = null; + DataType arrayType = null; for (Object currentValue : valueList) { checkParamValueValidity(entry, classification, currentValue, loc, errors); DataType currentType = DataType.fromJava(currentValue); - if ((type != null) && (type != currentType)) { + if (currentType == DataType.NULL) { + errors.add( + new XContentParseException( + loc, + "Parameter [" + entry.getKey() + "] contains a null value. Null values are not allowed for multivalues" + ) + ); + continue; + } else if (arrayType != null && arrayType != currentType) { errors.add( new XContentParseException( loc, - entry.getKey() + " parameter has values from different types, found " + type + " and " + currentType + "Parameter [" + entry.getKey() + "] has values from different types, found " + arrayType + " and " + currentType ) ); - break; + continue; } - type = currentType; + arrayType = currentType; } return; } diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/10_basic.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/10_basic.yml index b9d20d4cd40cf..e38b661b2baa8 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/10_basic.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/10_basic.yml @@ -412,6 +412,38 @@ FROM EVAL SORT LIMIT with documents_found: - match: {values.1: ["1",2.0,null,true,123,1674835275193]} - match: {values.2: ["1",2.0,null,true,123,1674835275193]} +--- +"Test Unnamed, Multivalued Input Params": + - requires: + test_runner_features: [ capabilities ] + capabilities: + - method: POST + path: /_query + parameters: [ ] + capabilities: [ query_params_multi_values ] + reason: "multivalued parameters" + - do: + esql.query: + body: + query: 'from test | eval x = ?, y = ?, z = ?, t = ?, u = ? | keep x, y, z, t, u | limit 3' + params: [["1", "2"], [2.0, 3.0], [true, false], [123, 456], [1674835275193, 1674835275193]] + + - length: {columns: 5} + - match: {columns.0.name: "x"} + - match: {columns.0.type: "keyword"} + - match: {columns.1.name: "y"} + - match: {columns.1.type: "double"} + - match: {columns.2.name: "z"} + - match: {columns.2.type: "boolean"} + - match: {columns.3.name: "t"} + - match: {columns.3.type: "integer"} + - match: {columns.4.name: "u"} + - match: {columns.4.type: "long"} + - length: {values: 3} + - match: {values.0: [["1", "2"], [2.0, 3.0], [true, false], [123, 456], [1674835275193, 1674835275193]]} + - match: {values.1: [["1", "2"], [2.0, 3.0], [true, false], [123, 456], [1674835275193, 1674835275193]]} + - match: {values.2: [["1", "2"], [2.0, 3.0], [true, false], [123, 456], [1674835275193, 1674835275193]]} + --- "Test Unnamed Input Params Also For Limit And Sample": - requires: @@ -486,6 +518,39 @@ FROM EVAL SORT LIMIT with documents_found: - match: {values.1: ["1",2.0,null,true,123,1674835275193]} - match: {values.2: ["1",2.0,null,true,123,1674835275193]} +--- +"Test Named, Multivalued Input Params": + - requires: + test_runner_features: [ capabilities ] + capabilities: + - method: POST + path: /_query + parameters: [ ] + capabilities: [ parameter_for_limit, parameter_for_sample, query_params_multi_values ] + reason: "named or positional parameters, multivalued parameters" + + - do: + esql.query: + body: + query: 'from test | eval x = ?n1, y = ?n2, z = ?n3, t = ?n4, u = ?n5 | keep x, y, z, t, u | limit ?l' + params: [{"n1" : ["1", "2"]}, {"n2" : [2.0, 3.0]}, {"n3" : [true, false]}, {"n4" : [123, 456]}, {"n5": [1674835275193, -1674835275193]}, {"l": 3}] + + - length: {columns: 5} + - match: {columns.0.name: "x"} + - match: {columns.0.type: "keyword"} + - match: {columns.1.name: "y"} + - match: {columns.1.type: "double"} + - match: {columns.2.name: "z"} + - match: {columns.2.type: "boolean"} + - match: {columns.3.name: "t"} + - match: {columns.3.type: "integer"} + - match: {columns.4.name: "u"} + - match: {columns.4.type: "long"} + - length: {values: 3} + - match: {values.0: [["1", "2"], [2.0, 3.0], [true, false] ,[123, 456], [1674835275193, -1674835275193]]} + - match: {values.1: [["1", "2"], [2.0, 3.0], [true, false] ,[123, 456], [1674835275193, -1674835275193]]} + - match: {values.2: [["1", "2"], [2.0, 3.0], [true, false] ,[123, 456], [1674835275193, -1674835275193]]} + --- "Test Interval in Input Params": - requires: From b13d467575ea3d1050375d273b75764aff9ca2ee Mon Sep 17 00:00:00 2001 From: cdelgado Date: Wed, 10 Sep 2025 11:04:10 +0200 Subject: [PATCH 11/20] Fix tests --- .../xpack/esql/qa/rest/RestEsqlTestCase.java | 12 +-- .../xpack/esql/action/RequestXContent.java | 4 +- .../esql/action/EsqlQueryRequestTests.java | 75 +++++++++++++------ 3 files changed, 59 insertions(+), 32 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java index 5ffa5c913833c..467c3f66094dc 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java @@ -725,12 +725,12 @@ public void testArrayValuesNullsNotAllowedInValueParams() throws IOException { ); String error = EntityUtils.toString(re.getResponse().getEntity()).replaceAll("\\\\\n\s+\\\\", ""); - assertThat(error, containsString("[1:79] Parameter [a] contains a null value. Null values are not allowed for multivalues;")); - assertThat(error, containsString("[1:101] Parameter [b] contains a null value. Null values are not allowed for multivalues;")); - assertThat(error, containsString("[1:120] Parameter [c] contains a null value. Null values are not allowed for multivalues;")); - assertThat(error, containsString("[1:142] Parameter [d] contains a null value. Null values are not allowed for multivalues;")); - assertThat(error, containsString("[1:162] Parameter [e] contains a null value. Null values are not allowed for multivalues;")); - assertThat(error, containsString("[1:192] Parameter [f] contains a null value. Null values are not allowed for multivalues;")); + assertThat(error, containsString("[1:79] Parameter [a] contains a null value. Null values are not allowed for multivalues")); + assertThat(error, containsString("[1:101] Parameter [b] contains a null value. Null values are not allowed for multivalues")); + assertThat(error, containsString("[1:120] Parameter [c] contains a null value. Null values are not allowed for multivalues")); + assertThat(error, containsString("[1:142] Parameter [d] contains a null value. Null values are not allowed for multivalues")); + assertThat(error, containsString("[1:162] Parameter [e] contains a null value. Null values are not allowed for multivalues")); + assertThat(error, containsString("[1:192] Parameter [f] contains a null value. Null values are not allowed for multivalues")); } public void testArrayValuesAllowedInUnnamedParams() throws IOException { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java index f72ab0887600e..fdacf138506af 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java @@ -375,7 +375,7 @@ private static void checkParamValueValidity( "Parameter [" + entry.getKey() + "] contains a null value. Null values are not allowed for multivalues" ) ); - continue; + break; } else if (arrayType != null && arrayType != currentType) { errors.add( new XContentParseException( @@ -383,7 +383,7 @@ private static void checkParamValueValidity( "Parameter [" + entry.getKey() + "] has values from different types, found " + arrayType + " and " + currentType ) ); - continue; + break; } arrayType = currentType; } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index 94b4bd0d4fd8a..866a72081f8c5 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -44,7 +44,6 @@ import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Locale; @@ -109,7 +108,7 @@ public void testNamedParams() throws IOException { {"_n1" : "8.15.0"}, { "__n2" : 0.05}, {"__3" : -799810013}, {"__4n" : "127.0.0.1"}, {"_n5" : "esql"}, {"_n6" : null}, {"_n7" : false}, {"_n8": ["8.15.0", "8.19.0"]}, {"_n9": ["x", "y"]}, {"_n10": [true, false]}, {"_n11": [1.0, 1.1, 1.2]}, - {"_n12": [-799810013, 0, 799810013]}, {"_n13": [null, null, null]} + {"_n12": [-799810013, 0, 799810013]} ] }"""; List params = List.of( @@ -131,8 +130,7 @@ public void testNamedParams() throws IOException { new QueryParam("_n9", List.of("x", "y"), KEYWORD, ParserUtils.ParamClassification.VALUE), new QueryParam("_n10", List.of(true, false), BOOLEAN, ParserUtils.ParamClassification.VALUE), new QueryParam("_n11", List.of(1.0, 1.1, 1.2), DOUBLE, ParserUtils.ParamClassification.VALUE), - new QueryParam("_n12", List.of(-799810013, 0, 799810013), DataType.INTEGER, ParserUtils.ParamClassification.VALUE), - new QueryParam("_n13", Arrays.asList(null, null, null), NULL, ParserUtils.ParamClassification.VALUE) + new QueryParam("_n12", List.of(-799810013, 0, 799810013), DataType.INTEGER, ParserUtils.ParamClassification.VALUE) // TODO add mixed null values, or check all elements, and separate into a new method ); String json = String.format(Locale.ROOT, """ @@ -166,7 +164,7 @@ public void testNamedMultivaluedParams() throws IOException { String paramsString = """ ,"params":[ {"_n1": ["8.15.0", "8.19.0"]}, {"_n2": ["x", "y"]}, {"_n3": [true, false]}, {"_n4": [1.0, 1.1, 1.2]}, - {"_n5": [-799810013, 0, 799810013]}, {"_n6": [null, null, null]} + {"_n5": [-799810013, 0, 799810013]} ] }"""; List params = List.of( @@ -174,8 +172,7 @@ public void testNamedMultivaluedParams() throws IOException { new QueryParam("_n2", List.of("x", "y"), KEYWORD, ParserUtils.ParamClassification.VALUE), new QueryParam("_n3", List.of(true, false), BOOLEAN, ParserUtils.ParamClassification.VALUE), new QueryParam("_n4", List.of(1.0, 1.1, 1.2), DOUBLE, ParserUtils.ParamClassification.VALUE), - new QueryParam("_n5", List.of(-799810013, 0, 799810013), DataType.INTEGER, ParserUtils.ParamClassification.VALUE), - new QueryParam("_n6", Arrays.asList(null, null, null), NULL, ParserUtils.ParamClassification.VALUE) + new QueryParam("_n5", List.of(-799810013, 0, 799810013), DataType.INTEGER, ParserUtils.ParamClassification.VALUE) ); String json = String.format(Locale.ROOT, """ { @@ -336,23 +333,23 @@ public void testInvalidMultivaluedParams() throws IOException { Exception e1 = expectThrows(XContentParseException.class, () -> parseEsqlQueryRequestSync(json1)); assertThat( e1.getCause().getMessage(), - containsString("Failed to parse params: [3:2] _n1 parameter has values from different types, found NULL and KEYWORD") + containsString(" [3:2] Parameter [_n1] contains a null value. Null values are not allowed for multivalues;") ); assertThat( e1.getCause().getMessage(), - containsString("[3:29] _n2 parameter has values from different types, found NULL and KEYWORD") + containsString("[3:29] Parameter [_n2] contains a null value. Null values are not allowed for multivalues;") ); assertThat( e1.getCause().getMessage(), - containsString("[3:57] _n3 parameter has values from different types, found NULL and BOOLEAN") + containsString("[3:57] Parameter [_n3] contains a null value. Null values are not allowed for multivalues;") ); assertThat( e1.getCause().getMessage(), - containsString("[4:2] _n4 parameter has values from different types, found NULL and DOUBLE") + containsString("[4:2] Parameter [_n4] contains a null value. Null values are not allowed for multivalues;") ); assertThat( e1.getCause().getMessage(), - containsString("[4:30] _n5 parameter has values from different types, found NULL and INTEGER") + containsString("[4:30] Parameter [_n5] contains a null value. Null values are not allowed for multivalues;") ); assertThat(e1.getCause().getMessage(), containsString("[5:2] n6=[{value={a5=v5}}] is not supported as a parameter")); assertThat(e1.getCause().getMessage(), containsString("[5:40] n7=[{identifier=[x, y]}] is not supported as a parameter")); @@ -388,18 +385,48 @@ public void testInvalidParamsForIdentifiersPatterns() throws IOException { message, containsString("[2:15] [v] is not a valid param attribute, a valid attribute is any of VALUE, IDENTIFIER, PATTERN; ") ); - assertThat(message, containsString("[2:38] [n2] has multiple param attributes [identifier, pattern],")); - assertThat(message, containsString("only one of VALUE, IDENTIFIER, PATTERN can be defined in a param;")); - assertThat(message, containsString("[2:38] [v2] is not a valid value for PATTERN parameter,")); - assertThat(message, containsString("a valid value for PATTERN parameter is a string and contains *;")); - assertThat(message, containsString("[3:1] [n3] has multiple param attributes [identifier, pattern],")); - assertThat(message, containsString("only one of VALUE, IDENTIFIER, PATTERN can be defined in a param;")); - assertThat(message, containsString("[3:1] [v3] is not a valid value for PATTERN parameter,")); - assertThat(message, containsString("a valid value for PATTERN parameter is a string and contains *;")); - assertThat(message, containsString("[3:51] [n4] has multiple param attributes [pattern, value],")); - assertThat(message, containsString("only one of VALUE, IDENTIFIER, PATTERN can be defined in a param;")); - assertThat(message, containsString("[3:51] [v4.1] is not a valid value for PATTERN parameter,")); - assertThat(message, containsString("a valid value for PATTERN parameter is a string and contains *;")); + assertThat( + message, + containsString( + "[2:38] [n2] has multiple param attributes [identifier, pattern], " + + "only one of VALUE, IDENTIFIER, PATTERN can be defined in a param;" + ) + ); + assertThat( + message, + containsString( + "[2:38] [v2] is not a valid value for PATTERN parameter, " + + "a valid value for PATTERN parameter is a string and contains *;" + ) + ); + assertThat( + message, + containsString( + "[3:1] [n3] has multiple param attributes [identifier, pattern], " + + "only one of VALUE, IDENTIFIER, PATTERN can be defined in a param;" + ) + ); + assertThat( + message, + containsString( + "[3:1] [v3] is not a valid value for PATTERN parameter, " + + "a valid value for PATTERN parameter is a string and contains *;" + ) + ); + assertThat( + message, + containsString( + "[3:51] [n4] has multiple param attributes [pattern, value], " + + "only one of VALUE, IDENTIFIER, PATTERN can be defined in a param;" + ) + ); + assertThat( + message, + containsString( + "[3:51] [v4.1] is not a valid value for PATTERN parameter, " + + "a valid value for PATTERN parameter is a string and contains *;" + ) + ); assertThat(message, containsString("[4:1] n5={value={a5=v5}} is not supported as a parameter;")); assertThat(message, containsString("[4:36] [{a6.1=v6.1, a6.2=v6.2}] is not a valid value for IDENTIFIER parameter,")); assertThat(message, containsString("a valid value for IDENTIFIER parameter is a string;")); From 21c222a4819e82b65299fc12500b9e0f78670278 Mon Sep 17 00:00:00 2001 From: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com> Date: Wed, 10 Sep 2025 11:16:14 +0200 Subject: [PATCH 12/20] Update docs/changelog/134317.yaml --- docs/changelog/134317.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/changelog/134317.yaml b/docs/changelog/134317.yaml index c06fa0709cfaa..83b16bcb38b30 100644 --- a/docs/changelog/134317.yaml +++ b/docs/changelog/134317.yaml @@ -1,5 +1,5 @@ pr: 134317 summary: ES|QL - Allow multivalued query parameters -area: "ES|QL" +area: ES|QL type: enhancement -issues: [127556] +issues: [] From 5ddf1bfda47d537bab0ff2c0aade31cfd5164d3a Mon Sep 17 00:00:00 2001 From: cdelgado Date: Wed, 10 Sep 2025 11:20:35 +0200 Subject: [PATCH 13/20] Update changelog --- docs/changelog/134317.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/134317.yaml b/docs/changelog/134317.yaml index 83b16bcb38b30..7dea737b61b8a 100644 --- a/docs/changelog/134317.yaml +++ b/docs/changelog/134317.yaml @@ -2,4 +2,4 @@ pr: 134317 summary: ES|QL - Allow multivalued query parameters area: ES|QL type: enhancement -issues: [] +issues: [127556, 126710] From 7722600aba531cd6da3fd90bc5d091bfa2128e7c Mon Sep 17 00:00:00 2001 From: cdelgado Date: Wed, 10 Sep 2025 11:28:02 +0200 Subject: [PATCH 14/20] Undo EvalMapper change --- .../xpack/esql/evaluator/EvalMapper.java | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/EvalMapper.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/EvalMapper.java index 949e7d8b7f250..81f58cd2f24a0 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/EvalMapper.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/EvalMapper.java @@ -273,18 +273,7 @@ private static Block block(Literal lit, BlockFactory blockFactory, int positions if (multiValue.isEmpty()) { return blockFactory.newConstantNullBlock(positions); } - ElementType type = ElementType.NULL; - for (Object elem : multiValue) { - if (elem != null) { - type = ElementType.fromJava(elem.getClass()); - break; - } - } - if (type == ElementType.NULL) { - // all values are null - return blockFactory.newConstantNullBlock(positions); - } - var wrapper = BlockUtils.wrapperFor(blockFactory, type, positions); + var wrapper = BlockUtils.wrapperFor(blockFactory, ElementType.fromJava(multiValue.get(0).getClass()), positions); for (int i = 0; i < positions; i++) { wrapper.accept(multiValue); } From 8aa4a9a50fbeb1de61fb1c79491d88fbcae19212 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 10 Sep 2025 10:36:12 +0000 Subject: [PATCH 15/20] [CI] Auto commit changes from spotless --- .../org/elasticsearch/xpack/esql/action/EsqlCapabilities.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index dc7671cefa819..f3e6c8a341457 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -1474,7 +1474,6 @@ public enum Cap { */ QUERY_PARAMS_MULTI_VALUES(Build.current().isSnapshot()); - private final boolean enabled; Cap() { From 6d7e94ec1a0200edb564dbddcbe9babb6779ab92 Mon Sep 17 00:00:00 2001 From: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com> Date: Wed, 10 Sep 2025 14:53:00 +0200 Subject: [PATCH 16/20] Update docs/changelog/134317.yaml --- docs/changelog/134317.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/134317.yaml b/docs/changelog/134317.yaml index 7dea737b61b8a..83b16bcb38b30 100644 --- a/docs/changelog/134317.yaml +++ b/docs/changelog/134317.yaml @@ -2,4 +2,4 @@ pr: 134317 summary: ES|QL - Allow multivalued query parameters area: ES|QL type: enhancement -issues: [127556, 126710] +issues: [] From f1909da144cd62672ee19049bbe78df58199e0ba Mon Sep 17 00:00:00 2001 From: cdelgado Date: Thu, 11 Sep 2025 08:02:04 +0200 Subject: [PATCH 17/20] Fix QueryParam classification in test --- .../java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java index 4bc0d5e6b66f4..7035793304af8 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java @@ -145,7 +145,7 @@ public void testDenseVectorQueryParams() { Arrays.fill(queryVector, 0); EsqlQueryRequest queryRequest = new EsqlQueryRequest(); QueryParams queryParams = new QueryParams( - List.of(new QueryParam("queryVector", Arrays.asList(queryVector), DataType.INTEGER, ParserUtils.ParamClassification.PATTERN)) + List.of(new QueryParam("queryVector", Arrays.asList(queryVector), DataType.INTEGER, ParserUtils.ParamClassification.VALUE)) ); queryRequest.params(queryParams); From a846f9aa5883b9234b2a233c582eedc4049c2fc4 Mon Sep 17 00:00:00 2001 From: cdelgado Date: Thu, 11 Sep 2025 11:54:38 +0200 Subject: [PATCH 18/20] Improve error messages --- .../xpack/esql/qa/rest/RestEsqlTestCase.java | 43 +++++++++-- .../xpack/esql/action/RequestXContent.java | 74 ++++++++++++------- .../esql/action/EsqlQueryRequestTests.java | 64 ++++++++++++++-- 3 files changed, 144 insertions(+), 37 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java index 467c3f66094dc..65aa1e5df6ebb 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java @@ -725,12 +725,43 @@ public void testArrayValuesNullsNotAllowedInValueParams() throws IOException { ); String error = EntityUtils.toString(re.getResponse().getEntity()).replaceAll("\\\\\n\s+\\\\", ""); - assertThat(error, containsString("[1:79] Parameter [a] contains a null value. Null values are not allowed for multivalues")); - assertThat(error, containsString("[1:101] Parameter [b] contains a null value. Null values are not allowed for multivalues")); - assertThat(error, containsString("[1:120] Parameter [c] contains a null value. Null values are not allowed for multivalues")); - assertThat(error, containsString("[1:142] Parameter [d] contains a null value. Null values are not allowed for multivalues")); - assertThat(error, containsString("[1:162] Parameter [e] contains a null value. Null values are not allowed for multivalues")); - assertThat(error, containsString("[1:192] Parameter [f] contains a null value. Null values are not allowed for multivalues")); + assertThat( + error, + containsString( + "[1:79] Parameter [a] contains a null entry: [null, a2]. " + "Null values are not allowed in multivalued params;" + ) + ); + assertThat( + error, + containsString( + "[1:101] Parameter [b] contains a null entry: [null, 2]. " + "Null values are not allowed in multivalued params;" + ) + ); + assertThat( + error, + containsString( + "[1:120] Parameter [c] contains a null entry: [null, false]. " + "Null values are not allowed in multivalued params;" + ) + ); + assertThat( + error, + containsString( + "[1:142] Parameter [d] contains a null entry: [null, 2.2]. " + "Null values are not allowed in multivalued params;" + ) + ); + assertThat( + error, + containsString( + "[1:162] Parameter [e] contains a null entry: [null, 1674835275193]. " + + "Null values are not allowed in multivalued params;" + ) + ); + assertThat( + error, + containsString( + "[1:192] Parameter [f] contains a null entry: [null, null]. " + "Null values are not allowed in multivalued params" + ) + ); } public void testArrayValuesAllowedInUnnamedParams() throws IOException { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java index fdacf138506af..df3ba240c2d6d 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java @@ -195,26 +195,24 @@ private static QueryParams parseParams(XContentParser p) throws IOException { } } else { if (token == XContentParser.Token.START_ARRAY) { - DataType arrayType = null; + DataType arrayType = DataType.NULL; List paramValues = new ArrayList<>(); + boolean nullValueFound = false, mixedTypesFound = false; while ((p.nextToken()) != XContentParser.Token.END_ARRAY) { ParamValueAndType valueAndDataType = parseSingleParamValue(p, errors); DataType currentType = valueAndDataType.type; - if (currentType == DataType.NULL) { - errors.add(new XContentParseException(loc, "Unnamed parameter contains a null in a multivalued value")); - continue; + nullValueFound = nullValueFound | (currentType == DataType.NULL); + mixedTypesFound = mixedTypesFound | (arrayType != DataType.NULL && arrayType != currentType); + if (currentType != DataType.NULL) { + arrayType = currentType; } - if (arrayType != null && arrayType != currentType) { - errors.add( - new XContentParseException( - loc, - "Unnamed parameter has values from different types, found " + arrayType + " and " + currentType - ) - ); - } - arrayType = currentType; paramValues.add(valueAndDataType.value); } + if (nullValueFound) { + addNullEntryError(errors, loc, null, paramValues); + } else if (mixedTypesFound) { + addMixedTypesError(errors, loc, null, paramValues); + } unNamedParams.add(new QueryParam(null, paramValues, arrayType, VALUE)); } else { ParamValueAndType valueAndDataType = parseSingleParamValue(p, errors); @@ -243,6 +241,42 @@ private static QueryParams parseParams(XContentParser p) throws IOException { return new QueryParams(namedParams.isEmpty() ? unNamedParams : namedParams); } + private static void addMixedTypesError( + List errors, + XContentLocation loc, + String paramName, + List paramValues + ) { + errors.add( + new XContentParseException( + loc, + "Parameter " + + (paramName == null ? "" : "[" + paramName + "] ") + + "contains mixed data types: " + + paramValues + + ". Mixed data types are not allowed in multivalued params." + ) + ); + } + + private static void addNullEntryError( + List errors, + XContentLocation loc, + String paramName, + List paramValues + ) { + errors.add( + new XContentParseException( + loc, + "Parameter " + + (paramName == null ? "" : "[" + paramName + "] ") + + "contains a null entry: " + + paramValues + + ". Null values are not allowed in multivalued params" + ) + ); + } + private record ParamValueAndType(Object value, DataType type) {} private static ParamValueAndType parseSingleParamValue(XContentParser p, List errors) throws IOException { @@ -369,20 +403,10 @@ private static void checkParamValueValidity( checkParamValueValidity(entry, classification, currentValue, loc, errors); DataType currentType = DataType.fromJava(currentValue); if (currentType == DataType.NULL) { - errors.add( - new XContentParseException( - loc, - "Parameter [" + entry.getKey() + "] contains a null value. Null values are not allowed for multivalues" - ) - ); + addNullEntryError(errors, loc, entry.getKey(), valueList); break; } else if (arrayType != null && arrayType != currentType) { - errors.add( - new XContentParseException( - loc, - "Parameter [" + entry.getKey() + "] has values from different types, found " + arrayType + " and " + currentType - ) - ); + addMixedTypesError(errors, loc, entry.getKey(), valueList); break; } arrayType = currentType; diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index 866a72081f8c5..b77ab2d903672 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -308,7 +308,7 @@ public void testInvalidParams() throws IOException { ); } - public void testInvalidMultivaluedParams() throws IOException { + public void testInvalidMultivaluedNamedParams() throws IOException { String query = randomAlphaOfLengthBetween(1, 100); boolean columnar = randomBoolean(); Locale locale = randomLocale(random()); @@ -333,29 +333,81 @@ public void testInvalidMultivaluedParams() throws IOException { Exception e1 = expectThrows(XContentParseException.class, () -> parseEsqlQueryRequestSync(json1)); assertThat( e1.getCause().getMessage(), - containsString(" [3:2] Parameter [_n1] contains a null value. Null values are not allowed for multivalues;") + containsString( + "[3:2] Parameter [_n1] contains a null entry: [null, 8.15.0]. Null values are not allowed in multivalued params;" + ) ); assertThat( e1.getCause().getMessage(), - containsString("[3:29] Parameter [_n2] contains a null value. Null values are not allowed for multivalues;") + containsString( + "[3:29] Parameter [_n2] contains a null entry: [null, null, x]. Null values are not allowed in multivalued params;" + ) ); assertThat( e1.getCause().getMessage(), - containsString("[3:57] Parameter [_n3] contains a null value. Null values are not allowed for multivalues;") + containsString( + "[3:57] Parameter [_n3] contains a null entry: [null, true, false]. Null values are not allowed in multivalued params;" + ) ); assertThat( e1.getCause().getMessage(), - containsString("[4:2] Parameter [_n4] contains a null value. Null values are not allowed for multivalues;") + containsString( + "[4:2] Parameter [_n4] contains a null entry: [null, 1.0, null]. Null values are not allowed in multivalued params;" + ) ); assertThat( e1.getCause().getMessage(), - containsString("[4:30] Parameter [_n5] contains a null value. Null values are not allowed for multivalues;") + containsString( + "[4:30] Parameter [_n5] contains a null entry: [null, -799810013, null, 799810013]. " + + "Null values are not allowed in multivalued params;" + ) ); assertThat(e1.getCause().getMessage(), containsString("[5:2] n6=[{value={a5=v5}}] is not supported as a parameter")); assertThat(e1.getCause().getMessage(), containsString("[5:40] n7=[{identifier=[x, y]}] is not supported as a parameter")); assertThat(e1.getCause().getMessage(), containsString("[5:80] n8=[{pattern=[x*, y*]}] is not supported as a parameter")); } + public void testInvalidMultivaluedUnnamedParams() throws IOException { + String query = randomAlphaOfLengthBetween(1, 100); + boolean columnar = randomBoolean(); + Locale locale = randomLocale(random()); + QueryBuilder filter = randomQueryBuilder(); + + // invalid named parameter for multivalued constants + String paramsString = """ + "params":[ + [null, "8.15.0"], [null, null, "x"], [null, true, false], [null, 1.0, null], [null, -799810013, null, 799810013] + ]"""; + String json1 = String.format(Locale.ROOT, """ + { + %s, + "query": "%s", + "columnar": %s, + "locale": "%s", + "filter": %s + }""", paramsString, query, columnar, locale.toLanguageTag(), filter); + + Exception e1 = expectThrows(XContentParseException.class, () -> parseEsqlQueryRequestSync(json1)); + assertThat( + e1.getCause().getMessage(), + containsString("[3:2] Parameter contains a null entry: [null, 8.15.0]. Null values are not allowed in multivalued params;") + ); + assertThat( + e1.getCause().getMessage(), + containsString("[3:20] Parameter contains a null entry: [null, null, x]. Null values are not allowed in multivalued params;") + ); + assertThat( + e1.getCause().getMessage(), + containsString( + "[3:39] Parameter contains a null entry: [null, true, false]. Null values are not allowed in multivalued params;" + ) + ); + assertThat( + e1.getCause().getMessage(), + containsString("[3:60] Parameter contains a null entry: [null, 1.0, null]. Null values are not allowed in multivalued params;") + ); + } + public void testInvalidParamsForIdentifiersPatterns() throws IOException { String query = randomAlphaOfLengthBetween(1, 100); boolean columnar = randomBoolean(); From b9a03ff6d9d7c60a1a1ad56bf7d2180888c32c06 Mon Sep 17 00:00:00 2001 From: cdelgado Date: Thu, 11 Sep 2025 11:58:26 +0200 Subject: [PATCH 19/20] Remove snapshot from capability --- .../xpack/esql/qa/rest/RestEsqlTestCase.java | 8 -------- .../elasticsearch/xpack/esql/plugin/KnnFunctionIT.java | 5 ----- .../elasticsearch/xpack/esql/action/EsqlCapabilities.java | 3 +-- .../elasticsearch/xpack/esql/analysis/AnalyzerTests.java | 5 ----- 4 files changed, 1 insertion(+), 20 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java index 65aa1e5df6ebb..8c3c243de3e02 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java @@ -688,8 +688,6 @@ public void testErrorMessageForInvalidIntervalParams() throws IOException { } public void testArrayValuesAllowedInValueParams() throws IOException { - assumeTrue("multivalues for params", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled()); - Map responseMap = runEsql( RequestObjectBuilder.jsonBuilder() .query("row a = ?a | eval b = ?b, c = ?c, d = ?d, e = ?e") @@ -710,8 +708,6 @@ public void testArrayValuesAllowedInValueParams() throws IOException { } public void testArrayValuesNullsNotAllowedInValueParams() throws IOException { - assumeTrue("multivalues for params", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled()); - ResponseException re = expectThrows( ResponseException.class, () -> runEsqlSync( @@ -765,8 +761,6 @@ public void testArrayValuesNullsNotAllowedInValueParams() throws IOException { } public void testArrayValuesAllowedInUnnamedParams() throws IOException { - assumeTrue("multivalues for params", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled()); - Map responseMap = runEsql( RequestObjectBuilder.jsonBuilder() .query("row a = ? | eval b = ?, c = ?, d = ?, e = ?") @@ -794,8 +788,6 @@ public void testArrayValuesAllowedInUnnamedParams() throws IOException { } public void testErrorMessageForArrayValuesInNonValueParams() throws IOException { - assumeTrue("multivalues for params", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled()); - ResponseException re = expectThrows( ResponseException.class, () -> runEsql( diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java index 7035793304af8..c9efb5c3ae409 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java @@ -136,11 +136,6 @@ public void testKnnOptions() { } public void testDenseVectorQueryParams() { - assumeTrue( - "multi values for query params capability must be available", - EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled() - ); - float[] queryVector = new float[numDims]; Arrays.fill(queryVector, 0); EsqlQueryRequest queryRequest = new EsqlQueryRequest(); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index dc7671cefa819..8d599ebe9252e 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -1472,8 +1472,7 @@ public enum Cap { /** * Multivalued query parameters */ - QUERY_PARAMS_MULTI_VALUES(Build.current().isSnapshot()); - + QUERY_PARAMS_MULTI_VALUES(); private final boolean enabled; diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java index 57f6ab47a8416..826f33764222c 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java @@ -2375,11 +2375,6 @@ private static void checkDenseVectorCastingKnn(String fieldName) { } public void testDenseVectorImplicitCastingKnnQueryParams() { - assumeTrue( - "multi values for query params capability must be available", - EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled() - ); - checkDenseVectorCastingKnnQueryParams("float_vector"); checkDenseVectorCastingKnnQueryParams("byte_vector"); } From 205f92067a0bdcec78a35736155c02aade0826b4 Mon Sep 17 00:00:00 2001 From: cdelgado Date: Thu, 11 Sep 2025 12:54:47 +0200 Subject: [PATCH 20/20] Merge tests --- .../elasticsearch/xpack/esql/analysis/AnalyzerTests.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java index 27060d6bdcd3b..417eb0f1a7834 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java @@ -2421,9 +2421,9 @@ private void checkDenseVectorCastingKnnQueryParams(String fieldName) { var limit = as(plan, Limit.class); var filter = as(limit.child(), Filter.class); var knn = as(filter.condition(), Knn.class); - var queryVector = as(knn.query(), Literal.class); - assertEquals(DataType.DENSE_VECTOR, queryVector.dataType()); - assertThat(queryVector.value(), equalTo(List.of(0f, 1f, 2f))); + var queryVector = as(knn.query(), ToDenseVector.class); + var literal = as(queryVector.field(), Literal.class); + assertThat(literal.value(), equalTo(List.of(0, 1, 2))); } public void testDenseVectorImplicitCastingSimilarityFunctions() {