Skip to content

Commit dcfe053

Browse files
authored
Add validation step to quoted join query construction (elastic#125731)
* Add validation step to quoted join query construction * Fix double validation
1 parent 800e312 commit dcfe053

File tree

3 files changed

+35
-7
lines changed

3 files changed

+35
-7
lines changed

muted-tests.yml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -347,9 +347,6 @@ tests:
347347
- class: org.elasticsearch.index.shard.StoreRecoveryTests
348348
method: testAddIndices
349349
issue: https://github.com/elastic/elasticsearch/issues/124104
350-
- class: org.elasticsearch.xpack.esql.parser.StatementParserTests
351-
method: testInvalidJoinPatterns
352-
issue: https://github.com/elastic/elasticsearch/issues/125536
353350
- class: org.elasticsearch.xpack.inference.registry.ModelRegistryMetadataTests
354351
method: testUpgrade
355352
issue: https://github.com/elastic/elasticsearch/issues/125554

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.antlr.v4.runtime.Token;
1212
import org.antlr.v4.runtime.tree.ParseTree;
1313
import org.elasticsearch.Build;
14+
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
1415
import org.elasticsearch.common.Strings;
1516
import org.elasticsearch.core.Tuple;
1617
import org.elasticsearch.dissect.DissectException;
@@ -568,6 +569,13 @@ public PlanFactory visitJoinCommand(EsqlBaseParser.JoinCommandContext ctx) {
568569
rightPattern
569570
);
570571
}
572+
if (rightPattern.contains(IndexNameExpressionResolver.SelectorResolver.SELECTOR_SEPARATOR)) {
573+
throw new ParsingException(
574+
source(target),
575+
"invalid index pattern [{}], index pattern selectors are not supported in LOOKUP JOIN",
576+
rightPattern
577+
);
578+
}
571579

572580
UnresolvedRelation right = new UnresolvedRelation(
573581
source(target),

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

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
import static org.elasticsearch.xpack.esql.EsqlTestUtils.paramAsPattern;
8181
import static org.elasticsearch.xpack.esql.EsqlTestUtils.referenceAttribute;
8282
import static org.elasticsearch.xpack.esql.IdentifierGenerator.Features.CROSS_CLUSTER;
83+
import static org.elasticsearch.xpack.esql.IdentifierGenerator.Features.DATE_MATH;
8384
import static org.elasticsearch.xpack.esql.IdentifierGenerator.Features.INDEX_SELECTOR;
8485
import static org.elasticsearch.xpack.esql.IdentifierGenerator.Features.WILDCARD_PATTERN;
8586
import static org.elasticsearch.xpack.esql.IdentifierGenerator.randomIndexPattern;
@@ -3142,18 +3143,21 @@ public void testInvalidJoinPatterns() {
31423143
if (EsqlCapabilities.Cap.INDEX_COMPONENT_SELECTORS.isEnabled()) {
31433144
{
31443145
// Selectors are not supported on the left of the join query if used with cluster ids.
3145-
var fromPatterns = randomIndexPatterns(CROSS_CLUSTER);
3146+
// Unquoted case: The language specification does not allow mixing `:` and `::` characters in an index expression
3147+
var fromPatterns = randomIndexPatterns(CROSS_CLUSTER, without(DATE_MATH));
31463148
// We do different validation based on the quotation of the pattern
31473149
// Autogenerated patterns will not mix cluster ids with selectors. Unquote it to ensure stable tests
31483150
fromPatterns = unquoteIndexPattern(fromPatterns) + "::data";
31493151
var joinPattern = randomIndexPattern(without(CROSS_CLUSTER), without(WILDCARD_PATTERN), without(INDEX_SELECTOR));
31503152
expectError(
31513153
"FROM " + fromPatterns + " | LOOKUP JOIN " + joinPattern + " ON " + randomIdentifier(),
3152-
"mismatched input '::' expecting {<EOF>, '|', ',', 'metadata'}"
3154+
"mismatched input '::' expecting {"
31533155
);
31543156
}
31553157
{
31563158
// Selectors are not supported on the left of the join query if used with cluster ids.
3159+
// Quoted case: The language specification allows mixing `:` and `::` characters in a quoted expression, but this usage
3160+
// must cause a validation exception in the non-generated code.
31573161
var fromPatterns = randomIndexPatterns(CROSS_CLUSTER, without(INDEX_SELECTOR));
31583162
// We do different validation based on the quotation of the pattern
31593163
// Autogenerated patterns will not mix cluster ids with selectors. Unquote, modify, and requote it to ensure stable tests
@@ -3166,12 +3170,31 @@ public void testInvalidJoinPatterns() {
31663170
}
31673171
{
31683172
// Selectors are not yet supported in join patterns on the right.
3169-
var joinPattern = randomIndexPattern(without(CROSS_CLUSTER), without(WILDCARD_PATTERN), INDEX_SELECTOR);
3173+
// Unquoted case: The language specification does not allow mixing `:` and `::` characters in an index expression
3174+
var joinPattern = randomIndexPattern(without(CROSS_CLUSTER), without(WILDCARD_PATTERN), without(DATE_MATH), INDEX_SELECTOR);
3175+
// We do different validation based on the quotation of the pattern, so forcefully unquote the expression instead of leaving
3176+
// it to chance.
3177+
joinPattern = unquoteIndexPattern(joinPattern);
31703178
expectError(
3171-
"FROM " + randomIndexPatterns() + " | LOOKUP JOIN " + joinPattern + " ON " + randomIdentifier(),
3179+
"FROM " + randomIndexPatterns(without(CROSS_CLUSTER)) + " | LOOKUP JOIN " + joinPattern + " ON " + randomIdentifier(),
31723180
"extraneous input ':' expecting {QUOTED_STRING, UNQUOTED_SOURCE}"
31733181
);
31743182
}
3183+
{
3184+
// Selectors are not yet supported in join patterns on the right.
3185+
// Quoted case: The language specification allows `::` characters in a quoted expression, but this usage
3186+
// must cause a validation exception in the non-generated code.
3187+
var joinPattern = randomIndexPattern(without(CROSS_CLUSTER), without(WILDCARD_PATTERN), without(DATE_MATH), INDEX_SELECTOR);
3188+
// We do different validation based on the quotation of the pattern, so forcefully quote the expression instead of leaving
3189+
// it to chance.
3190+
joinPattern = "\"" + unquoteIndexPattern(joinPattern) + "\"";
3191+
expectError(
3192+
"FROM " + randomIndexPatterns(without(CROSS_CLUSTER)) + " | LOOKUP JOIN " + joinPattern + " ON " + randomIdentifier(),
3193+
"invalid index pattern ["
3194+
+ unquoteIndexPattern(joinPattern)
3195+
+ "], index pattern selectors are not supported in LOOKUP JOIN"
3196+
);
3197+
}
31753198
}
31763199
}
31773200

0 commit comments

Comments
 (0)