Skip to content

Commit ab1146d

Browse files
carlosdelestcarlosdelest
committed
Don't allow null in multivalues
1 parent ceccf94 commit ab1146d

File tree

3 files changed

+116
-32
lines changed

3 files changed

+116
-32
lines changed

‎x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java‎

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -692,10 +692,10 @@ public void testArrayValuesAllowedInValueParams() throws IOException {
692692

693693
Map<String, Object> responseMap = runEsql(
694694
RequestObjectBuilder.jsonBuilder()
695-
.query("row a = ?a | eval b = ?b, c = ?c, d = ?d, e = ?e, f = ?f")
695+
.query("row a = ?a | eval b = ?b, c = ?c, d = ?d, e = ?e")
696696
.params(
697697
"[{\"a\" : [\"a1\", \"a2\"]}, {\"b\" : [1, 2]}, {\"c\": [true, false]}, {\"d\": [1.1, 2.2]},"
698-
+ " {\"e\": [1674835275193, 1674835275193]}, {\"f\": [null, null]}]"
698+
+ " {\"e\": [1674835275193, 1674835275193]}]"
699699
)
700700
);
701701
System.out.println(responseMap);
@@ -706,28 +706,40 @@ public void testArrayValuesAllowedInValueParams() throws IOException {
706706
.item(matchesList().item(true).item(false))
707707
.item(matchesList().item(1.1).item(2.2))
708708
.item(matchesList().item(1674835275193L).item(1674835275193L))
709-
.item(null) // constant null block, no multi-value
710709
);
710+
}
711711

712-
assertResultMap(
713-
responseMap,
714-
matchesList().item(matchesMap().entry("name", "a").entry("type", "keyword"))
715-
.item(matchesMap().entry("name", "b").entry("type", "integer"))
716-
.item(matchesMap().entry("name", "c").entry("type", "boolean"))
717-
.item(matchesMap().entry("name", "d").entry("type", "double"))
718-
.item(matchesMap().entry("name", "e").entry("type", "long"))
719-
.item(matchesMap().entry("name", "f").entry("type", "null")),
720-
values
712+
public void testArrayValuesNullsNotAllowedInValueParams() throws IOException {
713+
assumeTrue("multivalues for params", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled());
714+
715+
ResponseException re = expectThrows(
716+
ResponseException.class,
717+
() -> runEsqlSync(
718+
RequestObjectBuilder.jsonBuilder()
719+
.query("row a = ?a | eval b = ?b, c = ?c, d = ?d, e = ?e, f = ?f")
720+
.params(
721+
"[{\"a\" : [null, \"a2\"]}, {\"b\" : [null, 2]}, {\"c\": [null, false]}, {\"d\": [null, 2.2]},"
722+
+ " {\"e\": [null, 1674835275193]}, {\"f\": [null, null]}]"
723+
)
724+
)
721725
);
726+
727+
String error = EntityUtils.toString(re.getResponse().getEntity()).replaceAll("\\\\\n\s+\\\\", "");
728+
assertThat(error, containsString("[1:79] Parameter [a] contains a null value. Null values are not allowed for multivalues;"));
729+
assertThat(error, containsString("[1:101] Parameter [b] contains a null value. Null values are not allowed for multivalues;"));
730+
assertThat(error, containsString("[1:120] Parameter [c] contains a null value. Null values are not allowed for multivalues;"));
731+
assertThat(error, containsString("[1:142] Parameter [d] contains a null value. Null values are not allowed for multivalues;"));
732+
assertThat(error, containsString("[1:162] Parameter [e] contains a null value. Null values are not allowed for multivalues;"));
733+
assertThat(error, containsString("[1:192] Parameter [f] contains a null value. Null values are not allowed for multivalues;"));
722734
}
723735

724736
public void testArrayValuesAllowedInUnnamedParams() throws IOException {
725737
assumeTrue("multivalues for params", EsqlCapabilities.Cap.QUERY_PARAMS_MULTI_VALUES.isEnabled());
726738

727739
Map<String, Object> responseMap = runEsql(
728740
RequestObjectBuilder.jsonBuilder()
729-
.query("row a = ? | eval b = ?, c = ?, d = ?, e = ?, f = ?")
730-
.params("[[\"a1\", \"a2\"], [1, 2], [true, false], [1.1, 2.2], [1674835275193, 1674835275193], [null, null]]")
741+
.query("row a = ? | eval b = ?, c = ?, d = ?, e = ?")
742+
.params("[[\"a1\", \"a2\"], [1, 2], [true, false], [1.1, 2.2], [1674835275193, 1674835275193]]")
731743
);
732744
System.out.println(responseMap);
733745

@@ -737,7 +749,6 @@ public void testArrayValuesAllowedInUnnamedParams() throws IOException {
737749
.item(matchesList().item(true).item(false))
738750
.item(matchesList().item(1.1).item(2.2))
739751
.item(matchesList().item(1674835275193L).item(1674835275193L))
740-
.item(null)
741752
);
742753

743754
assertResultMap(
@@ -746,8 +757,7 @@ public void testArrayValuesAllowedInUnnamedParams() throws IOException {
746757
.item(matchesMap().entry("name", "b").entry("type", "integer"))
747758
.item(matchesMap().entry("name", "c").entry("type", "boolean"))
748759
.item(matchesMap().entry("name", "d").entry("type", "double"))
749-
.item(matchesMap().entry("name", "e").entry("type", "long"))
750-
.item(matchesMap().entry("name", "f").entry("type", "null")),
760+
.item(matchesMap().entry("name", "e").entry("type", "long")),
751761
values
752762
);
753763
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RequestXContent.java‎

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -195,26 +195,27 @@ private static QueryParams parseParams(XContentParser p) throws IOException {
195195
}
196196
} else {
197197
if (token == XContentParser.Token.START_ARRAY) {
198-
DataType currentType = null;
198+
DataType arrayType = null;
199199
List<Object> paramValues = new ArrayList<>();
200200
while ((p.nextToken()) != XContentParser.Token.END_ARRAY) {
201201
ParamValueAndType valueAndDataType = parseSingleParamValue(p, errors);
202-
if (currentType == null) {
203-
currentType = valueAndDataType.type;
204-
} else if (currentType != valueAndDataType.type) {
202+
DataType currentType = valueAndDataType.type;
203+
if (currentType == DataType.NULL) {
204+
errors.add(new XContentParseException(loc, "Unnamed parameter contains a null in a multivalued value"));
205+
continue;
206+
}
207+
if (arrayType != null && arrayType != currentType) {
205208
errors.add(
206209
new XContentParseException(
207210
loc,
208-
"Unnamed parameter has values from different types, found "
209-
+ currentType
210-
+ " and "
211-
+ valueAndDataType.type
211+
"Unnamed parameter has values from different types, found " + arrayType + " and " + currentType
212212
)
213213
);
214214
}
215+
arrayType = currentType;
215216
paramValues.add(valueAndDataType.value);
216217
}
217-
unNamedParams.add(new QueryParam(null, paramValues, currentType, VALUE));
218+
unNamedParams.add(new QueryParam(null, paramValues, arrayType, VALUE));
218219
} else {
219220
ParamValueAndType valueAndDataType = parseSingleParamValue(p, errors);
220221
unNamedParams.add(new QueryParam(null, valueAndDataType.value, valueAndDataType.type, VALUE));
@@ -246,7 +247,7 @@ private record ParamValueAndType(Object value, DataType type) {}
246247

247248
private static ParamValueAndType parseSingleParamValue(XContentParser p, List<XContentParseException> errors) throws IOException {
248249
Object paramValue = null;
249-
DataType type = null;
250+
DataType type = DataType.NULL;
250251
XContentParser.Token token = p.currentToken();
251252
if (token == XContentParser.Token.VALUE_STRING) {
252253
paramValue = p.text();
@@ -363,20 +364,28 @@ private static void checkParamValueValidity(
363364
return;
364365
}
365366
// Multivalued field
366-
DataType type = null;
367+
DataType arrayType = null;
367368
for (Object currentValue : valueList) {
368369
checkParamValueValidity(entry, classification, currentValue, loc, errors);
369370
DataType currentType = DataType.fromJava(currentValue);
370-
if ((type != null) && (type != currentType)) {
371+
if (currentType == DataType.NULL) {
372+
errors.add(
373+
new XContentParseException(
374+
loc,
375+
"Parameter [" + entry.getKey() + "] contains a null value. Null values are not allowed for multivalues"
376+
)
377+
);
378+
continue;
379+
} else if (arrayType != null && arrayType != currentType) {
371380
errors.add(
372381
new XContentParseException(
373382
loc,
374-
entry.getKey() + " parameter has values from different types, found " + type + " and " + currentType
383+
"Parameter [" + entry.getKey() + "] has values from different types, found " + arrayType + " and " + currentType
375384
)
376385
);
377-
break;
386+
continue;
378387
}
379-
type = currentType;
388+
arrayType = currentType;
380389
}
381390
return;
382391
}

‎x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/10_basic.yml‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,38 @@ FROM EVAL SORT LIMIT with documents_found:
412412
- match: {values.1: ["1",2.0,null,true,123,1674835275193]}
413413
- match: {values.2: ["1",2.0,null,true,123,1674835275193]}
414414

415+
---
416+
"Test Unnamed, Multivalued Input Params":
417+
- requires:
418+
test_runner_features: [ capabilities ]
419+
capabilities:
420+
- method: POST
421+
path: /_query
422+
parameters: [ ]
423+
capabilities: [ query_params_multi_values ]
424+
reason: "multivalued parameters"
425+
- do:
426+
esql.query:
427+
body:
428+
query: 'from test | eval x = ?, y = ?, z = ?, t = ?, u = ? | keep x, y, z, t, u | limit 3'
429+
params: [["1", "2"], [2.0, 3.0], [true, false], [123, 456], [1674835275193, 1674835275193]]
430+
431+
- length: {columns: 5}
432+
- match: {columns.0.name: "x"}
433+
- match: {columns.0.type: "keyword"}
434+
- match: {columns.1.name: "y"}
435+
- match: {columns.1.type: "double"}
436+
- match: {columns.2.name: "z"}
437+
- match: {columns.2.type: "boolean"}
438+
- match: {columns.3.name: "t"}
439+
- match: {columns.3.type: "integer"}
440+
- match: {columns.4.name: "u"}
441+
- match: {columns.4.type: "long"}
442+
- length: {values: 3}
443+
- match: {values.0: [["1", "2"], [2.0, 3.0], [true, false], [123, 456], [1674835275193, 1674835275193]]}
444+
- match: {values.1: [["1", "2"], [2.0, 3.0], [true, false], [123, 456], [1674835275193, 1674835275193]]}
445+
- match: {values.2: [["1", "2"], [2.0, 3.0], [true, false], [123, 456], [1674835275193, 1674835275193]]}
446+
415447
---
416448
"Test Unnamed Input Params Also For Limit And Sample":
417449
- requires:
@@ -486,6 +518,39 @@ FROM EVAL SORT LIMIT with documents_found:
486518
- match: {values.1: ["1",2.0,null,true,123,1674835275193]}
487519
- match: {values.2: ["1",2.0,null,true,123,1674835275193]}
488520

521+
---
522+
"Test Named, Multivalued Input Params":
523+
- requires:
524+
test_runner_features: [ capabilities ]
525+
capabilities:
526+
- method: POST
527+
path: /_query
528+
parameters: [ ]
529+
capabilities: [ parameter_for_limit, parameter_for_sample, query_params_multi_values ]
530+
reason: "named or positional parameters, multivalued parameters"
531+
532+
- do:
533+
esql.query:
534+
body:
535+
query: 'from test | eval x = ?n1, y = ?n2, z = ?n3, t = ?n4, u = ?n5 | keep x, y, z, t, u | limit ?l'
536+
params: [{"n1" : ["1", "2"]}, {"n2" : [2.0, 3.0]}, {"n3" : [true, false]}, {"n4" : [123, 456]}, {"n5": [1674835275193, -1674835275193]}, {"l": 3}]
537+
538+
- length: {columns: 5}
539+
- match: {columns.0.name: "x"}
540+
- match: {columns.0.type: "keyword"}
541+
- match: {columns.1.name: "y"}
542+
- match: {columns.1.type: "double"}
543+
- match: {columns.2.name: "z"}
544+
- match: {columns.2.type: "boolean"}
545+
- match: {columns.3.name: "t"}
546+
- match: {columns.3.type: "integer"}
547+
- match: {columns.4.name: "u"}
548+
- match: {columns.4.type: "long"}
549+
- length: {values: 3}
550+
- match: {values.0: [["1", "2"], [2.0, 3.0], [true, false] ,[123, 456], [1674835275193, -1674835275193]]}
551+
- match: {values.1: [["1", "2"], [2.0, 3.0], [true, false] ,[123, 456], [1674835275193, -1674835275193]]}
552+
- match: {values.2: [["1", "2"], [2.0, 3.0], [true, false] ,[123, 456], [1674835275193, -1674835275193]]}
553+
489554
---
490555
"Test Interval in Input Params":
491556
- requires:

0 commit comments

Comments
 (0)