Skip to content

Commit b1398a7

Browse files
fang-xing-esqlfang-xing-esql
authored
[ES|QL] Simplify syntax of named parameter for identifier and pattern (#115061) (#115520)
* simplify syntax of named parameter for identifier and pattern (cherry picked from commit 92ecd36) # Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java
1 parent 73e6a85 commit b1398a7

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
@@ -673,7 +673,7 @@ public void testErrorMessageForArrayValuesInParams() throws IOException {
673673
public void testNamedParamsForIdentifierAndIdentifierPatterns() throws IOException {
674674
assumeTrue(
675675
"named parameters for identifiers and patterns require snapshot build",
676-
EsqlCapabilities.Cap.NAMED_PARAMETER_FOR_FIELD_AND_FUNCTION_NAMES.isEnabled()
676+
EsqlCapabilities.Cap.NAMED_PARAMETER_FOR_FIELD_AND_FUNCTION_NAMES_SIMPLIFIED_SYNTAX.isEnabled()
677677
);
678678
bulkLoadTestData(10);
679679
// positive
@@ -685,12 +685,9 @@ public void testNamedParamsForIdentifierAndIdentifierPatterns() throws IOExcepti
685685
)
686686
)
687687
.params(
688-
"[{\"n1\" : {\"value\" : \"integer\" , \"kind\" : \"identifier\"}},"
689-
+ "{\"n2\" : {\"value\" : \"short\" , \"kind\" : \"identifier\"}}, "
690-
+ "{\"n3\" : {\"value\" : \"double\" , \"kind\" : \"identifier\"}},"
691-
+ "{\"n4\" : {\"value\" : \"boolean\" , \"kind\" : \"identifier\"}}, "
692-
+ "{\"n5\" : {\"value\" : \"xx*\" , \"kind\" : \"pattern\"}}, "
693-
+ "{\"fn1\" : {\"value\" : \"max\" , \"kind\" : \"identifier\"}}]"
688+
"[{\"n1\" : {\"identifier\" : \"integer\"}}, {\"n2\" : {\"identifier\" : \"short\"}}, "
689+
+ "{\"n3\" : {\"identifier\" : \"double\"}}, {\"n4\" : {\"identifier\" : \"boolean\"}}, "
690+
+ "{\"n5\" : {\"pattern\" : \"xx*\"}}, {\"fn1\" : {\"identifier\" : \"max\"}}]"
694691
);
695692
Map<String, Object> result = runEsql(query);
696693
Map<String, String> colA = Map.of("name", "boolean", "type", "boolean");
@@ -729,10 +726,7 @@ public void testNamedParamsForIdentifierAndIdentifierPatterns() throws IOExcepti
729726
ResponseException.class,
730727
() -> runEsqlSync(
731728
requestObjectBuilder().query(format(null, "from {} | {}", testIndexName(), command.getKey()))
732-
.params(
733-
"[{\"n1\" : {\"value\" : \"integer\" , \"kind\" : \"identifier\"}},"
734-
+ "{\"n2\" : {\"value\" : \"short\" , \"kind\" : \"identifier\"}}]"
735-
)
729+
.params("[{\"n1\" : {\"identifier\" : \"integer\"}}, {\"n2\" : {\"identifier\" : \"short\"}}]")
736730
)
737731
);
738732
error = re.getMessage();
@@ -752,9 +746,8 @@ public void testNamedParamsForIdentifierAndIdentifierPatterns() throws IOExcepti
752746
() -> runEsqlSync(
753747
requestObjectBuilder().query(format(null, "from {} | {}", testIndexName(), command.getKey()))
754748
.params(
755-
"[{\"n1\" : {\"value\" : \"`n1`\" , \"kind\" : \"identifier\"}},"
756-
+ "{\"n2\" : {\"value\" : \"`n2`\" , \"kind\" : \"identifier\"}}, "
757-
+ "{\"n3\" : {\"value\" : \"`n3`\" , \"kind\" : \"identifier\"}}]"
749+
"[{\"n1\" : {\"identifier\" : \"`n1`\"}}, {\"n2\" : {\"identifier\" : \"`n2`\"}}, "
750+
+ "{\"n3\" : {\"identifier\" : \"`n3`\"}}]"
758751
)
759752
)
760753
);
@@ -782,7 +775,7 @@ public void testNamedParamsForIdentifierAndIdentifierPatterns() throws IOExcepti
782775
ResponseException.class,
783776
() -> runEsqlSync(
784777
requestObjectBuilder().query(format(null, "from {} | ?cmd {}", testIndexName(), command.getValue()))
785-
.params("[{\"cmd\" : {\"value\" : \"" + command.getKey() + "\", \"kind\" : \"identifier\"}}]")
778+
.params("[{\"cmd\" : {\"identifier\" : \"" + command.getKey() + "\"}}]")
786779
)
787780
);
788781
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
@@ -378,11 +378,6 @@ public enum Cap {
378378
*/
379379
DATE_DIFF_YEAR_CALENDARIAL,
380380

381-
/**
382-
* Support named parameters for field names.
383-
*/
384-
NAMED_PARAMETER_FOR_FIELD_AND_FUNCTION_NAMES(Build.current().isSnapshot()),
385-
386381
/**
387382
* Fix sorting not allowed on _source and counters.
388383
*/
@@ -402,7 +397,12 @@ public enum Cap {
402397
* Fix for an optimization that caused wrong results
403398
* https://github.com/elastic/elasticsearch/issues/115281
404399
*/
405-
FIX_FILTER_PUSHDOWN_PAST_STATS;
400+
FIX_FILTER_PUSHDOWN_PAST_STATS,
401+
402+
/**
403+
* Support simplified syntax for named parameters for field and function names.
404+
*/
405+
NAMED_PARAMETER_FOR_FIELD_AND_FUNCTION_NAMES_SIMPLIFIED_SYNTAX(Build.current().isSnapshot());
406406

407407
private final boolean enabled;
408408

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