Update grammar to rely on indexPattern instead of identifier in join target #120494
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| pr: 120494 | ||
| summary: Update grammar to rely on `indexPattern` instead of identifier in join target | ||
| area: ES|QL | ||
| type: enhancement | ||
| issues: [] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -328,7 +328,7 @@ joinCommand | |
| ; | ||
|
|
||
| joinTarget | ||
| : index=indexPattern (AS alias=identifier)? | ||
| ; | ||
|
|
||
| joinCondition | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,6 +62,8 @@ | |
| import org.elasticsearch.xpack.esql.plan.logical.Rename; | ||
| import org.elasticsearch.xpack.esql.plan.logical.Row; | ||
| import org.elasticsearch.xpack.esql.plan.logical.UnresolvedRelation; | ||
| import org.elasticsearch.xpack.esql.plan.logical.join.JoinTypes; | ||
| import org.elasticsearch.xpack.esql.plan.logical.join.LookupJoin; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.HashMap; | ||
|
|
@@ -2939,4 +2941,69 @@ public void testNamedFunctionArgumentWithUnsupportedNamedParameterTypes() { | |
| ); | ||
| } | ||
| } | ||
|
|
||
| public void testValidJoinPattern() { | ||
|
There was a problem hiding this comment. Using the index pattern makes sense for parser (vs grammar) validation however I would have expected for patterns to be rejected from the start instead of waiting to resolve them first. There was a problem hiding this comment. Added in 2b642b9 |
||
| var basePattern = randomIndexPatterns(); | ||
| var joinPattern = randomIndexPattern(); | ||
| var onField = randomIdentifier(); | ||
| var type = randomFrom("", "LOOKUP "); | ||
|
|
||
|
Comment on lines
+2954
to
+2955
There was a problem hiding this comment. The empty string looks wrong - Consider removing the plain join, but it's also fine as-is due to this only testing the parsing itself. |
||
| var plan = statement("FROM " + basePattern + " | " + type + " JOIN " + joinPattern + " ON " + onField); | ||
|
|
||
| var join = as(plan, LookupJoin.class); | ||
| assertThat(as(join.left(), UnresolvedRelation.class).table().index(), equalTo(unquote(basePattern))); | ||
| assertThat(as(join.right(), UnresolvedRelation.class).table().index(), equalTo(unquote(joinPattern))); | ||
|
|
||
| var joinType = as(join.config().type(), JoinTypes.UsingJoinType.class); | ||
| assertThat(joinType.columns(), hasSize(1)); | ||
| assertThat(as(joinType.columns().getFirst(), UnresolvedAttribute.class).name(), equalTo(onField)); | ||
| assertThat(joinType.coreJoin().joinName(), equalTo("LEFT OUTER")); | ||
| } | ||
|
|
||
| private static String randomIndexPatterns() { | ||
| return maybeQuote(String.join(",", randomList(1, 5, StatementParserTests::randomIndexPattern))); | ||
| } | ||
|
|
||
| private static String randomIndexPattern() { | ||
| String pattern = maybeQuote(randomIndexIdentifier()); | ||
| if (randomBoolean()) { | ||
| var cluster = randomIdentifier(); | ||
| pattern = maybeQuote(cluster + ":" + pattern); | ||
| } | ||
| return pattern; | ||
| } | ||
|
|
||
| /** | ||
| * This generates random valid index, alias or pattern | ||
| */ | ||
| private static String randomIndexIdentifier() { | ||
| // https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-create-index.html#indices-create-api-path-params | ||
| var validFirstCharacters = "abcdefghijklmnopqrstuvwxyz0123456789!'$^&"; | ||
| var validCharacters = validFirstCharacters + "+-_."; | ||
|
|
||
| var index = new StringBuilder(); | ||
| if (randomInt(9) == 0) {// hidden index | ||
| index.append('.'); | ||
| } | ||
| index.append(randomCharacterFrom(validFirstCharacters)); | ||
| for (int i = 0; i < randomIntBetween(1, 100); i++) { | ||
| index.append(randomCharacterFrom(validCharacters)); | ||
| } | ||
| if (randomBoolean()) {// pattern | ||
| index.append('*'); | ||
| } | ||
| return index.toString(); | ||
| } | ||
|
|
||
| private static char randomCharacterFrom(String str) { | ||
| return str.charAt(randomInt(str.length() - 1)); | ||
| } | ||
|
|
||
| private static String maybeQuote(String term) { | ||
| return randomBoolean() && term.contains("\"") == false ? "\"" + term + "\"" : term; | ||
| } | ||
|
|
||
| private static String unquote(String term) { | ||
| return term.replace("\"", ""); | ||
| } | ||
| } | ||
ivancea marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add randomization to have the index specified with and without double quotes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to keep this test simple. All possible index name variations are covered in dedicated parser test.