Skip to content
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
e534c8b
First version
carlosdelest Sep 8, 2025
a30d9ae
Update docs/changelog/134317.yaml
carlosdelest Sep 8, 2025
3112666
[CI] Auto commit changes from spotless
Sep 8, 2025
f4f0678
Fix changelog
carlosdelest Sep 9, 2025
cc4d17e
Fix param validation
carlosdelest Sep 9, 2025
a72a293
Add REST tests and fix parsing
carlosdelest Sep 9, 2025
1dd6669
Spotless
carlosdelest Sep 9, 2025
2c95a66
Add capabilities guard
carlosdelest Sep 9, 2025
ceccf94
Handle null multivalues
carlosdelest Sep 9, 2025
ab1146d
Don't allow null in multivalues
carlosdelest Sep 10, 2025
b13d467
Fix tests
carlosdelest Sep 10, 2025
21c222a
Update docs/changelog/134317.yaml
carlosdelest Sep 10, 2025
5ddf1bf
Update changelog
carlosdelest Sep 10, 2025
7722600
Undo EvalMapper change
carlosdelest Sep 10, 2025
019c66e
Merge remote-tracking branch 'origin/main' into enhancement/esql-mult…
carlosdelest Sep 10, 2025
8aa4a9a
[CI] Auto commit changes from spotless
Sep 10, 2025
6d7e94e
Update docs/changelog/134317.yaml
carlosdelest Sep 10, 2025
f1909da
Fix QueryParam classification in test
carlosdelest Sep 11, 2025
a846f9a
Improve error messages
carlosdelest Sep 11, 2025
b9a03ff
Remove snapshot from capability
carlosdelest Sep 11, 2025
6bbc621
Merge remote-tracking branch 'carlosdelest/enhancement/esql-multi-val…
carlosdelest Sep 11, 2025
cdbb701
Merge remote-tracking branch 'origin/main' into enhancement/esql-mult…
carlosdelest Sep 11, 2025
205f920
Merge tests
carlosdelest Sep 11, 2025
e199aef
Merge branch 'main' into enhancement/esql-multi-valued-params
carlosdelest Sep 11, 2025
1117aff
Merge branch 'main' into enhancement/esql-multi-valued-params
carlosdelest Sep 11, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/134317.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 134317
summary: ES|QL - Allow multivalued query parameters
area: ES|QL
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 -> {
Copy link
Member Author

Choose a reason for hiding this comment

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

Add data type resolution for lists

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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -687,14 +687,102 @@ public void testErrorMessageForInvalidIntervalParams() throws IOException {
);
}

public void testErrorMessageForArrayValuesInParams() throws IOException {
public void testArrayValuesAllowedInValueParams() throws IOException {
assumeTrue("multivalues for params", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled());

Map<String, Object> responseMap = runEsql(
RequestObjectBuilder.jsonBuilder()
.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]}]"
)
);
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))
);
}

public void testArrayValuesNullsNotAllowedInValueParams() throws IOException {
assumeTrue("multivalues for params", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled());

ResponseException re = expectThrows(
ResponseException.class,
() -> runEsql(RequestObjectBuilder.jsonBuilder().query("row a = 1 | eval x = ?").params("[{\"n1\": [5, 6, 7]}]"))
() -> 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 {
assumeTrue("multivalues for params", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled());

Map<String, Object> responseMap = runEsql(
RequestObjectBuilder.jsonBuilder()
.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);

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))
);

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")),
values
);
}

public void testErrorMessageForArrayValuesInNonValueParams() throws IOException {
assumeTrue("multivalues for params", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled());

ResponseException re = expectThrows(
ResponseException.class,
() -> 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")
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -130,6 +135,38 @@ 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))
Copy link
Member

Choose a reason for hiding this comment

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

Is queryVector a constant value? Why this param set as a PATTERN?

Copy link
Member Author

@carlosdelest carlosdelest Sep 11, 2025

Choose a reason for hiding this comment

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

My bad - fixed in f1909da

);

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<List<Object>> valuesList = EsqlTestUtils.getValuesList(resp);
assertEquals(5, valuesList.size());
}
}

public void testKnnNonPushedDown() {
float[] queryVector = new float[numDims];
Arrays.fill(queryVector, 0.0f);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1467,7 +1467,12 @@ public enum Cap {
/**
* Support for the Present function
*/
FN_PRESENT;
FN_PRESENT,

/**
* Multivalued query parameters
*/
QUERY_PARAMS_MULTI_VALUES(Build.current().isSnapshot());
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm thinking on not making this available just for snapshots, but would like to hear concerns about it

Copy link
Member

Choose a reason for hiding this comment

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

I think the best practice is to remove snapshot in a followup PR right?

Copy link
Member

Choose a reason for hiding this comment

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

We might want to give a heads up to kibana, and add test-release label if this is behind snapshot.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the best practice is to remove snapshot in a followup PR right?

We tend to do that when there's incomplete work or we have to make a decision about it being useful or not as it is. I think this doesn't need to be hidden behind snapshot, as it's useful and even if we want to improve it in the future we can do out of snapshot.

Happy to hear other thoughts!

Copy link
Member Author

Choose a reason for hiding this comment

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

I've removed the snapshot capability in b9a03ff - this change will go on all builds


private final boolean enabled;

Expand Down
Loading