Skip to content

Commit 92ecd36

Browse files
fang-xing-esqlfang-xing-esql
authored
[ES|QL] Simplify syntax of named parameter for identifier and pattern (#115061)
* simplify syntax of named parameter for identifier and pattern
1 parent 2f64d20 commit 92ecd36

File tree

8 files changed

+122
-119
lines changed

8 files changed

+122
-119
lines changed

‎docs/changelog/115061.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 115061
2+
summary: "[ES|QL] Simplify syntax of named parameter for identifier and pattern"
3+
area: ES|QL
4+
type: bug
5+
issues: []

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

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,7 @@ public void testErrorMessageForArrayValuesInParams() throws IOException {
672672
public void testNamedParamsForIdentifierAndIdentifierPatterns() throws IOException {
673673
assumeTrue(
674674
"named parameters for identifiers and patterns require snapshot build",
675-
EsqlCapabilities.Cap.NAMED_PARAMETER_FOR_FIELD_AND_FUNCTION_NAMES.isEnabled()
675+
EsqlCapabilities.Cap.NAMED_PARAMETER_FOR_FIELD_AND_FUNCTION_NAMES_SIMPLIFIED_SYNTAX.isEnabled()
676676
);
677677
bulkLoadTestData(10);
678678
// positive
@@ -684,12 +684,9 @@ public void testNamedParamsForIdentifierAndIdentifierPatterns() throws IOExcepti
684684
)
685685
)
686686
.params(
687-
"[{\"n1\" : {\"value\" : \"integer\" , \"kind\" : \"identifier\"}},"
688-
+ "{\"n2\" : {\"value\" : \"short\" , \"kind\" : \"identifier\"}}, "
689-
+ "{\"n3\" : {\"value\" : \"double\" , \"kind\" : \"identifier\"}},"
690-
+ "{\"n4\" : {\"value\" : \"boolean\" , \"kind\" : \"identifier\"}}, "
691-
+ "{\"n5\" : {\"value\" : \"xx*\" , \"kind\" : \"pattern\"}}, "
692-
+ "{\"fn1\" : {\"value\" : \"max\" , \"kind\" : \"identifier\"}}]"
687+
"[{\"n1\" : {\"identifier\" : \"integer\"}}, {\"n2\" : {\"identifier\" : \"short\"}}, "
688+
+ "{\"n3\" : {\"identifier\" : \"double\"}}, {\"n4\" : {\"identifier\" : \"boolean\"}}, "
689+
+ "{\"n5\" : {\"pattern\" : \"xx*\"}}, {\"fn1\" : {\"identifier\" : \"max\"}}]"
693690
);
694691
Map<String, Object> result = runEsql(query);
695692
Map<String, String> colA = Map.of("name", "boolean", "type", "boolean");
@@ -728,10 +725,7 @@ public void testNamedParamsForIdentifierAndIdentifierPatterns() throws IOExcepti
728725
ResponseException.class,
729726
() -> runEsqlSync(
730727
requestObjectBuilder().query(format(null, "from {} | {}", testIndexName(), command.getKey()))
731-
.params(
732-
"[{\"n1\" : {\"value\" : \"integer\" , \"kind\" : \"identifier\"}},"
733-
+ "{\"n2\" : {\"value\" : \"short\" , \"kind\" : \"identifier\"}}]"
734-
)
728+
.params("[{\"n1\" : {\"identifier\" : \"integer\"}}, {\"n2\" : {\"identifier\" : \"short\"}}]")
735729
)
736730
);
737731
error = re.getMessage();
@@ -751,9 +745,8 @@ public void testNamedParamsForIdentifierAndIdentifierPatterns() throws IOExcepti
751745
() -> runEsqlSync(
752746
requestObjectBuilder().query(format(null, "from {} | {}", testIndexName(), command.getKey()))
753747
.params(
754-
"[{\"n1\" : {\"value\" : \"`n1`\" , \"kind\" : \"identifier\"}},"
755-
+ "{\"n2\" : {\"value\" : \"`n2`\" , \"kind\" : \"identifier\"}}, "
756-
+ "{\"n3\" : {\"value\" : \"`n3`\" , \"kind\" : \"identifier\"}}]"
748+
"[{\"n1\" : {\"identifier\" : \"`n1`\"}}, {\"n2\" : {\"identifier\" : \"`n2`\"}}, "
749+
+ "{\"n3\" : {\"identifier\" : \"`n3`\"}}]"
757750
)
758751
)
759752
);
@@ -781,7 +774,7 @@ public void testNamedParamsForIdentifierAndIdentifierPatterns() throws IOExcepti
781774
ResponseException.class,
782775
() -> runEsqlSync(
783776
requestObjectBuilder().query(format(null, "from {} | ?cmd {}", testIndexName(), command.getValue()))
784-
.params("[{\"cmd\" : {\"value\" : \"" + command.getKey() + "\", \"kind\" : \"identifier\"}}]")
777+
.params("[{\"cmd\" : {\"identifier\" : \"" + command.getKey() + "\"}}]")
785778
)
786779
);
787780
error = re.getMessage();

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -384,11 +384,6 @@ public enum Cap {
384384
*/
385385
DATE_DIFF_YEAR_CALENDARIAL,
386386

387-
/**
388-
* Support named parameters for field names.
389-
*/
390-
NAMED_PARAMETER_FOR_FIELD_AND_FUNCTION_NAMES(Build.current().isSnapshot()),
391-
392387
/**
393388
* Fix sorting not allowed on _source and counters.
394389
*/
@@ -431,7 +426,12 @@ public enum Cap {
431426
/**
432427
* This enables 60_usage.yml "Basic ESQL usage....non-snapshot" version test. See also the previous capability.
433428
*/
434-
NON_SNAPSHOT_TEST_FOR_TELEMETRY(Build.current().isSnapshot() == false);
429+
NON_SNAPSHOT_TEST_FOR_TELEMETRY(Build.current().isSnapshot() == false),
430+
431+
/**
432+
* Support simplified syntax for named parameters for field and function names.
433+
*/
434+
NAMED_PARAMETER_FOR_FIELD_AND_FUNCTION_NAMES_SIMPLIFIED_SYNTAX(Build.current().isSnapshot());
435435

436436
private final boolean enabled;
437437

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

Lines changed: 51 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
package org.elasticsearch.xpack.esql.action;
99

1010
import org.elasticsearch.common.settings.Settings;
11-
import org.elasticsearch.common.util.Maps;
1211
import org.elasticsearch.core.TimeValue;
1312
import org.elasticsearch.index.query.AbstractQueryBuilder;
1413
import org.elasticsearch.xcontent.ObjectParser;
@@ -90,19 +89,6 @@ String fields() {
9089
private static final ObjectParser<EsqlQueryRequest, Void> SYNC_PARSER = objectParserSync(EsqlQueryRequest::syncEsqlQueryRequest);
9190
private static final ObjectParser<EsqlQueryRequest, Void> ASYNC_PARSER = objectParserAsync(EsqlQueryRequest::asyncEsqlQueryRequest);
9291

93-
private enum ParamParsingKey {
94-
VALUE,
95-
KIND
96-
}
97-
98-
private static final Map<String, ParamParsingKey> paramParsingKeys = Maps.newMapWithExpectedSize(ParamParsingKey.values().length);
99-
100-
static {
101-
for (ParamParsingKey e : ParamParsingKey.values()) {
102-
paramParsingKeys.put(e.name(), e);
103-
}
104-
}
105-
10692
/** Parses a synchronous request. */
10793
static EsqlQueryRequest parseSync(XContentParser parser) {
10894
return SYNC_PARSER.apply(parser, null);
@@ -180,25 +166,21 @@ private static QueryParams parseParams(XContentParser p) throws IOException {
180166
);
181167
}
182168
for (Map.Entry<String, Object> entry : param.fields.entrySet()) {
183-
ParserUtils.ParamClassification classification;
169+
ParserUtils.ParamClassification classification = null;
170+
paramValue = null;
184171
String paramName = entry.getKey();
185172
checkParamNameValidity(paramName, errors, loc);
186173

187-
if (EsqlCapabilities.Cap.NAMED_PARAMETER_FOR_FIELD_AND_FUNCTION_NAMES.isEnabled()
188-
&& entry.getValue() instanceof Map<?, ?> values) {// parameter specified as key:value pairs
189-
Map<ParamParsingKey, Object> paramElements = Maps.newMapWithExpectedSize(2);
190-
for (Object keyName : values.keySet()) {
191-
ParamParsingKey paramType = checkParamValueKeysValidity(keyName.toString(), errors, loc);
192-
if (paramType != null) {
193-
paramElements.put(paramType, values.get(keyName));
174+
if (EsqlCapabilities.Cap.NAMED_PARAMETER_FOR_FIELD_AND_FUNCTION_NAMES_SIMPLIFIED_SYNTAX.isEnabled()
175+
&& entry.getValue() instanceof Map<?, ?> value) {// parameter specified as a key:value pair
176+
checkParamValueSize(paramName, value, loc, errors);
177+
for (Object keyName : value.keySet()) {
178+
classification = getParamClassification(keyName.toString(), errors, loc);
179+
if (classification != null) {
180+
paramValue = value.get(keyName);
181+
checkParamValueValidity(classification, paramValue, loc, errors);
194182
}
195183
}
196-
paramValue = paramElements.get(ParamParsingKey.VALUE);
197-
if (paramValue == null && values.size() > 1) { // require non-null value for identifier and pattern
198-
errors.add(new XContentParseException(loc, "[" + entry + "] does not have a value specified"));
199-
}
200-
201-
classification = getClassificationForParam(paramElements, loc, errors);
202184
} else {// parameter specifies as a value only
203185
paramValue = entry.getValue();
204186
classification = VALUE;
@@ -280,51 +262,67 @@ private static void checkParamNameValidity(String name, List<XContentParseExcept
280262
}
281263
}
282264

283-
private static ParamParsingKey checkParamValueKeysValidity(
265+
private static void checkParamValueSize(
266+
String paramName,
267+
Map<?, ?> paramValue,
268+
XContentLocation loc,
269+
List<XContentParseException> errors
270+
) {
271+
if (paramValue.size() == 1) {
272+
return;
273+
}
274+
String errorMessage;
275+
if (paramValue.isEmpty()) {
276+
errorMessage = " has no valid param attribute";
277+
} else {
278+
errorMessage = " has multiple param attributes ["
279+
+ paramValue.keySet().stream().map(Object::toString).collect(Collectors.joining(", "))
280+
+ "]";
281+
}
282+
errors.add(
283+
new XContentParseException(
284+
loc,
285+
"["
286+
+ paramName
287+
+ "]"
288+
+ errorMessage
289+
+ ", only one of "
290+
+ Arrays.stream(ParserUtils.ParamClassification.values())
291+
.map(ParserUtils.ParamClassification::name)
292+
.collect(Collectors.joining(", "))
293+
+ " can be defined in a param"
294+
)
295+
);
296+
}
297+
298+
private static ParserUtils.ParamClassification getParamClassification(
284299
String paramKeyName,
285300
List<XContentParseException> errors,
286301
XContentLocation loc
287302
) {
288-
ParamParsingKey paramType = paramParsingKeys.get(paramKeyName.toUpperCase(Locale.ROOT));
303+
ParserUtils.ParamClassification paramType = paramClassifications.get(paramKeyName.toUpperCase(Locale.ROOT));
289304
if (paramType == null) {
290305
errors.add(
291306
new XContentParseException(
292307
loc,
293308
"["
294309
+ paramKeyName
295310
+ "] is not a valid param attribute, a valid attribute is any of "
296-
+ Arrays.stream(ParamParsingKey.values()).map(ParamParsingKey::name).collect(Collectors.joining(", "))
311+
+ Arrays.stream(ParserUtils.ParamClassification.values())
312+
.map(ParserUtils.ParamClassification::name)
313+
.collect(Collectors.joining(", "))
297314
)
298315
);
299316
}
300317
return paramType;
301318
}
302319

303-
private static ParserUtils.ParamClassification getClassificationForParam(
304-
Map<ParamParsingKey, Object> paramElements,
320+
private static void checkParamValueValidity(
321+
ParserUtils.ParamClassification classification,
322+
Object value,
305323
XContentLocation loc,
306324
List<XContentParseException> errors
307325
) {
308-
Object value = paramElements.get(ParamParsingKey.VALUE);
309-
Object kind = paramElements.get(ParamParsingKey.KIND);
310-
ParserUtils.ParamClassification classification = VALUE;
311-
if (kind != null) {
312-
classification = paramClassifications.get(kind.toString().toUpperCase(Locale.ROOT));
313-
if (classification == null) {
314-
errors.add(
315-
new XContentParseException(
316-
loc,
317-
"["
318-
+ kind
319-
+ "] is not a valid param kind, a valid kind is any of "
320-
+ Arrays.stream(ParserUtils.ParamClassification.values())
321-
.map(ParserUtils.ParamClassification::name)
322-
.collect(Collectors.joining(", "))
323-
)
324-
);
325-
}
326-
}
327-
328326
// If a param is an "identifier" or a "pattern", validate it is a string.
329327
// If a param is a "pattern", validate it contains *.
330328
if (classification == IDENTIFIER || classification == PATTERN) {
@@ -345,6 +343,5 @@ private static ParserUtils.ParamClassification getClassificationForParam(
345343
);
346344
}
347345
}
348-
return classification;
349346
}
350347
}

0 commit comments

Comments
 (0)