Skip to content

Commit b3edb8d

Browse files
jbaierajbaiera
committed
Make cluster name and selector exclusive of one another.
1 parent 8df5cc2 commit b3edb8d

File tree

4 files changed

+83
-30
lines changed

4 files changed

+83
-30
lines changed

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/IdentifierBuilder.java‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,14 @@ public String visitIndexPattern(List<EsqlBaseParser.IndexPatternContext> ctx) {
109109
}
110110
} else {
111111
validateClusterString(clusterString, c);
112+
// Do not allow selectors on remote cluster expressions until they are supported
113+
if (selectorString != null) {
114+
InvalidIndexNameException ie = new InvalidIndexNameException(
115+
reassembleIndexName(clusterString, indexPattern, selectorString),
116+
"Selectors are not yet supported on remote cluster patterns"
117+
);
118+
throw new ParsingException(ie, source(c), ie.getMessage());
119+
}
112120
}
113121
patterns.add(reassembleIndexName(clusterString, indexPattern, selectorString));
114122
});
@@ -144,11 +152,18 @@ private static void validateIndexPattern(String indexPattern, EsqlBaseParser.Ind
144152
// Strip spaces off first because validation checks are not written to handle them
145153
index = index.strip();
146154
if (isRemoteIndexName(index)) { // skip the validation if there is remote cluster
155+
// Ensure that there are no selectors as they are not yet supported
156+
if (index.contains(SELECTOR_SEPARATOR)) {
157+
IllegalArgumentException ie = new IllegalArgumentException(
158+
"Cannot specify remote cluster and selector in same pattern"
159+
);
160+
throw new ParsingException(ie, source(ctx), ie.getMessage());
161+
}
147162
continue;
148163
}
149164
try {
150165
Tuple<String, String> splitPattern = IndexNameExpressionResolver.splitSelectorExpression(index);
151-
if (splitPattern.v2() == null) {
166+
if (splitPattern.v2() != null) {
152167
index = splitPattern.v1();
153168
}
154169
} catch (InvalidIndexNameException e) {

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/IdentifierGenerator.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,7 @@ public static String randomIndexPattern(Feature... features) {
8989
if (canAdd(Features.CROSS_CLUSTER, features)) {
9090
var cluster = maybeQuote(randomIdentifier());
9191
pattern = maybeQuote(cluster + ":" + pattern);
92-
}
93-
if (EsqlCapabilities.Cap.INDEX_COMPONENT_SELECTORS.isEnabled() && canAdd(Features.INDEX_SELECTOR, features)) {
92+
} else if (EsqlCapabilities.Cap.INDEX_COMPONENT_SELECTORS.isEnabled() && canAdd(Features.INDEX_SELECTOR, features)) {
9493
var selector = ESTestCase.randomFrom(IndexComponentSelector.values());
9594
pattern = maybeQuote(pattern + "::" + selector.getKey());
9695
}

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,10 @@ void expectInvalidIndexNameErrorWithLineNumber(String query, String indexString,
165165
expectInvalidIndexNameErrorWithLineNumber(query, "\"" + indexString + "\"", lineNumber, indexString);
166166
}
167167

168+
void expectErrorWithLineNumber(String query, String indexString, String lineNumber, String error) {
169+
expectError(LoggerMessageFormat.format(null, query, indexString), lineNumber + error);
170+
}
171+
168172
void expectInvalidIndexNameErrorWithLineNumber(String query, String indexString, String lineNumber, String name) {
169173
expectError(LoggerMessageFormat.format(null, query, indexString), lineNumber + "Invalid index name [" + name);
170174
}
@@ -176,4 +180,8 @@ void expectInvalidIndexNameErrorWithLineNumber(String query, String indexString,
176180
void expectDateMathErrorWithLineNumber(String query, String arg, String lineNumber, String error) {
177181
expectError(LoggerMessageFormat.format(null, query, arg), lineNumber + error);
178182
}
183+
184+
void expectDoubleColonErrorWithLineNumber(String query, String indexString, int lineNumber) {
185+
expectError(LoggerMessageFormat.format(null, query, indexString), "line 1:" + lineNumber + ": mismatched input '::'");
186+
}
179187
}

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java‎

Lines changed: 58 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -503,35 +503,17 @@ public void testStringAsIndexPattern() {
503503
clusterAndIndexAsIndexPattern(command, "*:index*");
504504
clusterAndIndexAsIndexPattern(command, "*:*");
505505
if (EsqlCapabilities.Cap.INDEX_COMPONENT_SELECTORS.isEnabled()) {
506+
assertStringAsIndexPattern("foo::*", command + " foo::*");
506507
assertStringAsIndexPattern("foo::data", command + " foo::data");
507508
assertStringAsIndexPattern("foo::failures", command + " foo::failures");
508-
assertStringAsIndexPattern("foo::*", command + " foo::*");
509-
assertStringAsIndexPattern("cluster:foo::data", command + " cluster:foo::data");
510-
assertStringAsIndexPattern("cluster:foo::failures", command + " cluster:foo::failures");
511-
assertStringAsIndexPattern("cluster:foo::*", command + " cluster:foo::*");
512-
assertStringAsIndexPattern("foo::*", command + " foo::*");
513-
assertStringAsIndexPattern("cluster:foo::*", command + " cluster:\"foo\"::*");
514-
assertStringAsIndexPattern("cluster:foo::*", command + " \"cluster:foo\"::*");
515-
assertStringAsIndexPattern("cluster:foo::*", command + " \"cluster:foo::*\"");
516-
assertStringAsIndexPattern("cluster:foo::data", command + " cluster:\"foo\"::data");
517-
assertStringAsIndexPattern("cluster:foo::data", command + " \"cluster:foo\"::data");
518-
assertStringAsIndexPattern("cluster:foo::data", command + " \"cluster:foo::data\"");
519-
assertStringAsIndexPattern("cluster:foo::failures", command + " cluster:\"foo\"::failures");
520-
assertStringAsIndexPattern("cluster:foo::failures", command + " \"cluster:foo\"::failures");
521-
assertStringAsIndexPattern("cluster:foo::failures", command + " \"cluster:foo::failures\"");
509+
assertStringAsIndexPattern("cluster:foo::failures", command + " cluster:\"foo::failures\"");
522510
assertStringAsIndexPattern("*,-foo::*", command + " *, \"-foo\"::*");
523511
assertStringAsIndexPattern("*,-foo::*", command + " *, \"-foo::*\"");
524512
assertStringAsIndexPattern("*::*", command + " *::*");
525513
assertStringAsIndexPattern(
526514
"<logstash-{now/M{yyyy.MM}}>::*,<logstash-{now/d{yyyy.MM.dd|+12:00}}>::failures",
527515
command + " <logstash-{now/M{yyyy.MM}}>::*, \"<logstash-{now/d{yyyy.MM.dd|+12:00}}>\"::failures"
528516
);
529-
clusterAndIndexAsIndexPattern(command, "cluster:index::data");
530-
clusterAndIndexAsIndexPattern(command, "cluster:*::data");
531-
clusterAndIndexAsIndexPattern(command, "*:index::data");
532-
clusterAndIndexAsIndexPattern(command, "*:index::*");
533-
clusterAndIndexAsIndexPattern(command, "*:index*::*");
534-
clusterAndIndexAsIndexPattern(command, "*:*::*");
535517
}
536518
}
537519
}
@@ -618,11 +600,60 @@ public void testInvalidCharacterInIndexPattern() {
618600
if (EsqlCapabilities.Cap.INDEX_COMPONENT_SELECTORS.isEnabled() && command.contains("LOOKUP_🐔") == false) {
619601
expectInvalidIndexNameErrorWithLineNumber(command, "index::dat", lineNumber);
620602
expectInvalidIndexNameErrorWithLineNumber(command, "index::failure", lineNumber);
621-
expectError(LoggerMessageFormat.format(null, command, "\"index:::data\""), lineNumber + "Invalid index name [index:::data");
622-
expectError(
623-
LoggerMessageFormat.format(null, command, "\"index::::data\""),
624-
lineNumber + "Invalid index name [index::::data"
603+
604+
// Cluster name cannot be combined with selector yet.
605+
var parseLineNumber = command.contains("FROM") ? 6 : 9;
606+
expectDoubleColonErrorWithLineNumber(command, "cluster:foo::*", parseLineNumber + 11);
607+
expectDoubleColonErrorWithLineNumber(command, "cluster:foo::data", parseLineNumber + 11);
608+
expectDoubleColonErrorWithLineNumber(command, "cluster:foo::failures", parseLineNumber + 11);
609+
610+
expectDoubleColonErrorWithLineNumber(command, "cluster:\"foo\"::*", parseLineNumber + 13);
611+
expectDoubleColonErrorWithLineNumber(command, "cluster:\"foo\"::data", parseLineNumber + 13);
612+
expectDoubleColonErrorWithLineNumber(command, "cluster:\"foo\"::failures", parseLineNumber + 13);
613+
614+
// TODO: Edge case that will be invalidated in follow up (https://github.com/elastic/elasticsearch/issues/122651)
615+
// expectDoubleColonErrorWithLineNumber(command, "\"cluster:foo\"::*", parseLineNumber + 13);
616+
// expectDoubleColonErrorWithLineNumber(command, "\"cluster:foo\"::data", parseLineNumber + 13);
617+
// expectDoubleColonErrorWithLineNumber(command, "\"cluster:foo\"::failures", parseLineNumber + 13);
618+
619+
expectErrorWithLineNumber(
620+
command,
621+
"\"cluster:foo::*\"",
622+
lineNumber,
623+
"Cannot specify remote cluster and selector in same pattern"
624+
);
625+
expectErrorWithLineNumber(
626+
command,
627+
"\"cluster:foo::data\"",
628+
lineNumber,
629+
"Cannot specify remote cluster and selector in same pattern"
625630
);
631+
expectErrorWithLineNumber(
632+
command,
633+
"\"cluster:foo::failures\"",
634+
lineNumber,
635+
"Cannot specify remote cluster and selector in same pattern"
636+
);
637+
638+
// Wildcards
639+
expectDoubleColonErrorWithLineNumber(command, "cluster:*::data", parseLineNumber + 9);
640+
expectDoubleColonErrorWithLineNumber(command, "*:index::data", parseLineNumber + 7);
641+
expectDoubleColonErrorWithLineNumber(command, "*:index::*", parseLineNumber + 7);
642+
expectDoubleColonErrorWithLineNumber(command, "*:index*::*", parseLineNumber + 8);
643+
expectDoubleColonErrorWithLineNumber(command, "*:*::*", parseLineNumber + 3);
644+
645+
// Too many colons
646+
expectInvalidIndexNameErrorWithLineNumber(command, "\"index:::data\"", lineNumber, "index:", "must not contain ':'");
647+
expectInvalidIndexNameErrorWithLineNumber(
648+
command,
649+
"\"index::::data\"",
650+
lineNumber,
651+
"index::::data",
652+
"Invalid usage of :: separator"
653+
);
654+
655+
// TODO: Edge case that will be invalidated in follow up (https://github.com/elastic/elasticsearch/issues/122651)
656+
expectDoubleColonErrorWithLineNumber(command, "cluster:\"index,index2\"::failures", parseLineNumber + 22);
626657
}
627658
}
628659

@@ -660,8 +691,8 @@ public void testInvalidCharacterInIndexPattern() {
660691
command,
661692
"\"indexpattern, --index::data\"",
662693
commands.get(command),
663-
"-index::data",
664-
"must not contain ':'"
694+
"-index",
695+
"must not start with '_', '-', or '+'"
665696
);
666697
}
667698
}
@@ -3109,7 +3140,7 @@ public void testInvalidJoinPatterns() {
31093140
joinPattern = maybeQuote(joinPattern + "::garbage");
31103141
expectError(
31113142
"FROM " + randomIndexPatterns() + " | JOIN " + joinPattern + " ON " + randomIdentifier(),
3112-
"mismatched input ':' expecting {"
3143+
"mismatched input 'JOIN' expecting {"
31133144
);
31143145
}
31153146
}

0 commit comments

Comments
 (0)