Skip to content

Commit c75e012

Browse files
fang-xing-esqlfang-xing-esql
authored
[ES|QL] Double parameter markers for identifiers (elastic#122459) (elastic#124949)
* double parameter markers for identifiers (cherry picked from commit bd81312) # Conflicts: # x-pack/plugin/esql/src/main/antlr/EsqlBaseLexer.tokens # x-pack/plugin/esql/src/main/antlr/EsqlBaseParser.tokens # x-pack/plugin/esql/src/main/antlr/lexer/Enrich.g4 # x-pack/plugin/esql/src/main/antlr/lexer/Expression.g4 # x-pack/plugin/esql/src/main/antlr/lexer/MvExpand.g4 # x-pack/plugin/esql/src/main/antlr/lexer/Project.g4 # x-pack/plugin/esql/src/main/antlr/lexer/Rename.g4 # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlBaseLexer.interp # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlBaseLexer.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlBaseParser.interp # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlBaseParser.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlBaseParserBaseListener.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlBaseParserBaseVisitor.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlBaseParserListener.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlBaseParserVisitor.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java
1 parent 28aa350 commit c75e012

File tree

20 files changed

+3707
-2091
lines changed

20 files changed

+3707
-2091
lines changed

‎docs/changelog/122459.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 122459
2+
summary: Double parameter markers for identifiers
3+
area: ES|QL
4+
type: enhancement
5+
issues: []

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

Lines changed: 204 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.elasticsearch.xcontent.XContentType;
3434
import org.elasticsearch.xpack.esql.AssertWarnings;
3535
import org.elasticsearch.xpack.esql.EsqlTestUtils;
36+
import org.elasticsearch.xpack.esql.action.EsqlCapabilities;
3637
import org.junit.After;
3738
import org.junit.Assert;
3839
import org.junit.Before;
@@ -83,9 +84,13 @@ public abstract class RestEsqlTestCase extends ESRestTestCase {
8384

8485
private static final String MAPPING_ALL_TYPES;
8586

87+
private static final String MAPPING_ALL_TYPES_LOOKUP;
88+
8689
static {
8790
String properties = EsqlTestUtils.loadUtf8TextFile("/mapping-all-types.json");
8891
MAPPING_ALL_TYPES = "{\"mappings\": " + properties + "}";
92+
String settings = "{\"settings\" : {\"mode\" : \"lookup\"}";
93+
MAPPING_ALL_TYPES_LOOKUP = settings + ", " + "\"mappings\": " + properties + "}";
8994
}
9095

9196
private static final String DOCUMENT_TEMPLATE = """
@@ -802,6 +807,193 @@ public void testErrorMessageForMissingParams() throws IOException {
802807
);
803808
}
804809

810+
public void testDoubleParamsForIdentifiers() throws IOException {
811+
assumeTrue(
812+
"double parameters markers for identifiers requires snapshot build",
813+
EsqlCapabilities.Cap.DOUBLE_PARAMETER_MARKERS_FOR_IDENTIFIERS.isEnabled()
814+
);
815+
bulkLoadTestData(10);
816+
// positive
817+
// named double parameters
818+
var query = requestObjectBuilder().query(
819+
format(
820+
null,
821+
"from {} | eval x1 = ??n1 | where ??n2 == x1 | stats xx2 = ??fn1(??n3) by ??n4 | keep ??n4, ??n5 | sort ??n4",
822+
testIndexName()
823+
)
824+
)
825+
.params(
826+
"[{\"n1\" : \"integer\"}, {\"n2\" : \"short\"}, {\"n3\" : \"double\"}, {\"n4\" : \"boolean\"}, "
827+
+ "{\"n5\" : \"xx2\"}, {\"fn1\" : \"max\"}]"
828+
);
829+
validateResultsOfDoubleParametersForIdentifiers(query);
830+
831+
// positional double parameters
832+
query = requestObjectBuilder().query(
833+
format(
834+
null,
835+
"from {} | eval x1 = ??1 | where ??2 == x1 | stats xx2 = ??6(??3) by ??4 | keep ??4, ??5 | sort ??4",
836+
testIndexName()
837+
)
838+
)
839+
.params(
840+
"[{\"n1\" : \"integer\"}, {\"n2\" : \"short\"}, {\"n3\" : \"double\"}, {\"n4\" : \"boolean\"}, "
841+
+ "{\"n5\" : \"xx2\"}, {\"fn1\" : \"max\"}]"
842+
);
843+
validateResultsOfDoubleParametersForIdentifiers(query);
844+
845+
query = requestObjectBuilder().query(
846+
format(
847+
null,
848+
"from {} | eval x1 = ??1 | where ??2 == x1 | stats xx2 = ??6(??3) by ??4 | keep ??4, ??5 | sort ??4",
849+
testIndexName()
850+
)
851+
).params("[\"integer\", \"short\", \"double\", \"boolean\", \"xx2\", \"max\"]");
852+
validateResultsOfDoubleParametersForIdentifiers(query);
853+
854+
// anonymous double parameters
855+
query = requestObjectBuilder().query(
856+
format(null, "from {} | eval x1 = ?? | where ?? == x1 | stats xx2 = ??(??) by ?? | keep ??, ?? | sort ??", testIndexName())
857+
)
858+
.params(
859+
"[{\"n1\" : \"integer\"}, {\"n2\" : \"short\"}, {\"fn1\" : \"max\"}, {\"n3\" : \"double\"}, {\"n4\" : \"boolean\"}, "
860+
+ "{\"n4\" : \"boolean\"}, {\"n5\" : \"xx2\"}, {\"n4\" : \"boolean\"}]"
861+
);
862+
validateResultsOfDoubleParametersForIdentifiers(query);
863+
864+
query = requestObjectBuilder().query(
865+
format(null, "from {} | eval x1 = ?? | where ?? == x1 | stats xx2 = ??(??) by ?? | keep ??, ?? | sort ??", testIndexName())
866+
).params("[\"integer\", \"short\", \"max\", \"double\", \"boolean\", \"boolean\", \"xx2\", \"boolean\"]");
867+
validateResultsOfDoubleParametersForIdentifiers(query);
868+
869+
// missing params
870+
ResponseException re = expectThrows(
871+
ResponseException.class,
872+
() -> runEsqlSync(
873+
requestObjectBuilder().query(
874+
format(
875+
null,
876+
"from {} | eval x1 = ??n1 | where ??n2 == x1 | stats xx2 = max(??n3) by ??n4 | keep ??n4, ??n5 | sort ??n4",
877+
testIndexName()
878+
)
879+
).params("[]")
880+
)
881+
);
882+
String error = re.getMessage().replaceAll("\\\\\n\s+\\\\", "");
883+
assertThat(error, containsString("ParsingException"));
884+
assertThat(error, containsString("Unknown query parameter [n1]"));
885+
886+
// param inside backquote is not recognized as a param
887+
Map<String, Integer> commandsWithLineNumber = Map.ofEntries(
888+
entry("eval x1 = `??n1`", 33),
889+
entry("where `??n1` == 1", 29),
890+
entry("stats x = max(n2) by `??n1`", 44),
891+
entry("stats x = max(`??n1`) by n2", 37),
892+
entry("keep `??n1`", 28),
893+
entry("sort `??n1`", 28)
894+
);
895+
for (Map.Entry<String, Integer> command : commandsWithLineNumber.entrySet()) {
896+
re = expectThrows(
897+
ResponseException.class,
898+
() -> runEsqlSync(
899+
requestObjectBuilder().query(format(null, "from {} | {}", testIndexName(), command.getKey()))
900+
.params("[{\"n1\" : \"integer\"}, {\"n2\" : \"short\"}]")
901+
)
902+
);
903+
error = re.getMessage().replaceAll("\\\\\n\s+\\\\", "");
904+
assertThat(error, containsString("VerificationException"));
905+
assertThat(error, containsString("line 1:" + command.getValue() + ": Unknown column [??n1]"));
906+
}
907+
908+
commandsWithLineNumber = Map.ofEntries(
909+
entry("rename ??n1 as ??n2", 30),
910+
entry("enrich idx2 ON ??n1 WITH ??n2 = ??n3", 38),
911+
entry("keep ??n1", 28),
912+
entry("drop ??n1", 28)
913+
);
914+
for (Map.Entry<String, Integer> command : commandsWithLineNumber.entrySet()) {
915+
re = expectThrows(
916+
ResponseException.class,
917+
() -> runEsqlSync(
918+
requestObjectBuilder().query(format(null, "from {} | {}", testIndexName(), command.getKey()))
919+
.params("[{\"n1\" : \"`n1`\"}, {\"n2\" : \"`n2`\"}, {\"n3\" : \"`n3`\"}]")
920+
)
921+
);
922+
error = re.getMessage().replaceAll("\\\\\n\s+\\\\", "");
923+
assertThat(error, containsString("VerificationException"));
924+
assertThat(error, containsString("line 1:" + command.getValue() + ": Unknown column [`n1`]"));
925+
}
926+
927+
// param cannot be used as a command name
928+
Map<String, String> paramsAsCommandNames = Map.ofEntries(
929+
entry("eval", "x = 1"),
930+
entry("where", "x == 1"),
931+
entry("stats", "x = count(*)"),
932+
entry("keep", "x"),
933+
entry("drop", "x"),
934+
entry("rename", "x as y"),
935+
entry("sort", "x"),
936+
entry("dissect", "x \"%{foo}\""),
937+
entry("grok", "x \"%{WORD:foo}\""),
938+
entry("enrich", "idx2 ON x"),
939+
entry("mvExpand", "x")
940+
);
941+
for (Map.Entry<String, String> command : paramsAsCommandNames.entrySet()) {
942+
re = expectThrows(
943+
ResponseException.class,
944+
() -> runEsqlSync(
945+
requestObjectBuilder().query(format(null, "from {} | ??cmd {}", testIndexName(), command.getValue()))
946+
.params("[{\"cmd\" : \"" + command.getKey() + "\"}]")
947+
)
948+
);
949+
error = re.getMessage().replaceAll("\\\\\n\s+\\\\", "");
950+
assertThat(error, containsString("ParsingException"));
951+
assertThat(error, containsString("line 1:23: mismatched input '??cmd' expecting {"));
952+
}
953+
}
954+
955+
public void testDoubleParamsWithLookupJoin() throws IOException {
956+
assumeTrue(
957+
"double parameters markers for identifiers requires snapshot build",
958+
EsqlCapabilities.Cap.DOUBLE_PARAMETER_MARKERS_FOR_IDENTIFIERS.isEnabled()
959+
);
960+
bulkLoadTestDataLookupMode(10);
961+
var query = requestObjectBuilder().query(
962+
format(
963+
null,
964+
"from {} | eval x1 = ??n1 | where ??n2 == x1 | lookup join {} on ??n3 | keep ??n4 | sort ??n4",
965+
testIndexName(),
966+
testIndexName()
967+
)
968+
).params("[{\"n1\" : \"integer\"}, {\"n2\" : \"short\"}, {\"n3\" : \"double\"}, {\"n4\" : \"boolean\"}]");
969+
Map<String, Object> result = runEsql(query);
970+
Map<String, String> colA = Map.of("name", "boolean", "type", "boolean");
971+
assertEquals(List.of(colA), result.get("columns"));
972+
assertEquals(
973+
List.of(
974+
List.of(false),
975+
List.of(false),
976+
List.of(false),
977+
List.of(false),
978+
List.of(false),
979+
List.of(true),
980+
List.of(true),
981+
List.of(true),
982+
List.of(true),
983+
List.of(true)
984+
),
985+
result.get("values")
986+
);
987+
}
988+
989+
private void validateResultsOfDoubleParametersForIdentifiers(RequestObjectBuilder query) throws IOException {
990+
Map<String, Object> result = runEsql(query);
991+
Map<String, String> colA = Map.of("name", "boolean", "type", "boolean");
992+
Map<String, String> colB = Map.of("name", "xx2", "type", "double");
993+
assertEquals(List.of(colA, colB), result.get("columns"));
994+
assertEquals(List.of(List.of(false, 9.1), List.of(true, 8.1)), result.get("values"));
995+
}
996+
805997
public void testErrorMessageForLiteralDateMathOverflow() throws IOException {
806998
List<String> dateMathOverflowExpressions = List.of(
807999
"2147483647 day + 1 day",
@@ -1386,13 +1578,22 @@ private static void bulkLoadTestData(int count) throws IOException {
13861578
bulkLoadTestData(count, 0, true, RestEsqlTestCase::createDocument);
13871579
}
13881580

1581+
private static void bulkLoadTestDataLookupMode(int count) throws IOException {
1582+
createIndex(testIndexName(), true);
1583+
bulkLoadTestData(count, 0, false, RestEsqlTestCase::createDocument);
1584+
}
1585+
1586+
private static void createIndex(String indexName, boolean lookupMode) throws IOException {
1587+
Request request = new Request("PUT", "/" + indexName);
1588+
request.setJsonEntity(lookupMode ? MAPPING_ALL_TYPES_LOOKUP : MAPPING_ALL_TYPES);
1589+
assertEquals(200, client().performRequest(request).getStatusLine().getStatusCode());
1590+
}
1591+
13891592
private static void bulkLoadTestData(int count, int firstIndex, boolean createIndex, IntFunction<String> createDocument)
13901593
throws IOException {
13911594
Request request;
13921595
if (createIndex) {
1393-
request = new Request("PUT", "/" + testIndexName());
1394-
request.setJsonEntity(MAPPING_ALL_TYPES);
1395-
assertEquals(200, client().performRequest(request).getStatusLine().getStatusCode());
1596+
createIndex(testIndexName(), false);
13961597
}
13971598

13981599
if (count > 0) {

‎x-pack/plugin/esql/src/main/antlr/EsqlBaseLexer.g4‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,13 +218,20 @@ PERCENT : '%';
218218
LEFT_BRACES : '{';
219219
RIGHT_BRACES : '}';
220220

221+
DOUBLE_PARAMS: {this.isDevVersion()}? '??';
222+
221223
NESTED_WHERE : WHERE -> type(WHERE);
222224

223225
NAMED_OR_POSITIONAL_PARAM
224226
: PARAM (LETTER | UNDERSCORE) UNQUOTED_ID_BODY*
225227
| PARAM DIGIT+
226228
;
227229

230+
NAMED_OR_POSITIONAL_DOUBLE_PARAMS
231+
: DOUBLE_PARAMS (LETTER | UNDERSCORE) UNQUOTED_ID_BODY*
232+
| DOUBLE_PARAMS DIGIT+
233+
;
234+
228235
// Brackets are funny. We can happen upon a CLOSING_BRACKET in two ways - one
229236
// way is to start in an explain command which then shifts us to expression
230237
// mode. Thus, the two popModes on CLOSING_BRACKET. The other way could as
@@ -316,6 +323,8 @@ PROJECT_DOT: DOT -> type(DOT);
316323
PROJECT_COMMA : COMMA -> type(COMMA);
317324
PROJECT_PARAM : PARAM -> type(PARAM);
318325
PROJECT_NAMED_OR_POSITIONAL_PARAM : NAMED_OR_POSITIONAL_PARAM -> type(NAMED_OR_POSITIONAL_PARAM);
326+
PROJECT_DOUBLE_PARAMS : {this.isDevVersion()}? DOUBLE_PARAMS -> type(DOUBLE_PARAMS);
327+
PROJECT_NAMED_OR_POSITIONAL_DOUBLE_PARAMS : {this.isDevVersion()}? NAMED_OR_POSITIONAL_DOUBLE_PARAMS -> type(NAMED_OR_POSITIONAL_DOUBLE_PARAMS);
319328
320329
fragment UNQUOTED_ID_BODY_WITH_PATTERN
321330
: (LETTER | DIGIT | UNDERSCORE | ASTERISK)
@@ -351,6 +360,8 @@ RENAME_COMMA : COMMA -> type(COMMA);
351360
RENAME_DOT: DOT -> type(DOT);
352361
RENAME_PARAM : PARAM -> type(PARAM);
353362
RENAME_NAMED_OR_POSITIONAL_PARAM : NAMED_OR_POSITIONAL_PARAM -> type(NAMED_OR_POSITIONAL_PARAM);
363+
RENAME_DOUBLE_PARAMS : {this.isDevVersion()}? DOUBLE_PARAMS -> type(DOUBLE_PARAMS);
364+
RENAME_NAMED_OR_POSITIONAL_DOUBLE_PARAMS : {this.isDevVersion()}? NAMED_OR_POSITIONAL_DOUBLE_PARAMS -> type(NAMED_OR_POSITIONAL_DOUBLE_PARAMS);
354365
355366
AS : 'as';
356367
@@ -424,6 +435,8 @@ ENRICH_FIELD_QUOTED_IDENTIFIER
424435

425436
ENRICH_FIELD_PARAM : PARAM -> type(PARAM);
426437
ENRICH_FIELD_NAMED_OR_POSITIONAL_PARAM : NAMED_OR_POSITIONAL_PARAM -> type(NAMED_OR_POSITIONAL_PARAM);
438+
ENRICH_FIELD_DOUBLE_PARAMS : {this.isDevVersion()}? DOUBLE_PARAMS -> type(DOUBLE_PARAMS);
439+
ENRICH_FIELD_NAMED_OR_POSITIONAL_DOUBLE_PARAMS : {this.isDevVersion()}? NAMED_OR_POSITIONAL_DOUBLE_PARAMS -> type(NAMED_OR_POSITIONAL_DOUBLE_PARAMS);
427440

428441
ENRICH_FIELD_LINE_COMMENT
429442
: LINE_COMMENT -> channel(HIDDEN)
@@ -442,6 +455,8 @@ MVEXPAND_PIPE : PIPE -> type(PIPE), popMode;
442455
MVEXPAND_DOT: DOT -> type(DOT);
443456
MVEXPAND_PARAM : PARAM -> type(PARAM);
444457
MVEXPAND_NAMED_OR_POSITIONAL_PARAM : NAMED_OR_POSITIONAL_PARAM -> type(NAMED_OR_POSITIONAL_PARAM);
458+
MVEXPAND_DOUBLE_PARAMS : {this.isDevVersion()}? DOUBLE_PARAMS -> type(DOUBLE_PARAMS);
459+
MVEXPAND_NAMED_OR_POSITIONAL_DOUBLE_PARAMS : {this.isDevVersion()}? NAMED_OR_POSITIONAL_DOUBLE_PARAMS -> type(NAMED_OR_POSITIONAL_DOUBLE_PARAMS);
445460

446461
MVEXPAND_QUOTED_IDENTIFIER
447462
: QUOTED_IDENTIFIER -> type(QUOTED_IDENTIFIER)

0 commit comments

Comments
 (0)