-
Notifications
You must be signed in to change notification settings - Fork 25.5k
Resolve indices using original index pattern #134218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 10 commits
e0f5989
9e56546
e172c13
62cb8e9
d653020
21ccd63
24b7a4c
b1e75f4
3029bfb
58c8230
ee777a4
06bfcfa
3c9a40b
cf31115
e9a0e29
880f169
444217d
e5254ec
685830f
adaa593
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ | |
import org.elasticsearch.ElasticsearchStatusException; | ||
import org.elasticsearch.core.Tuple; | ||
import org.elasticsearch.plugins.Plugin; | ||
import org.elasticsearch.xpack.esql.VerificationException; | ||
import org.elasticsearch.xpack.esql.plan.logical.Enrich; | ||
|
||
import java.util.ArrayList; | ||
|
@@ -65,25 +66,30 @@ public void testQueryAgainstNonMatchingClusterWildcardPattern() { | |
|
||
// since this wildcarded expression does not resolve to a valid remote cluster, it is not considered | ||
// a cross-cluster search and thus should not throw a license error | ||
String q = "FROM xremote*:events"; | ||
{ | ||
String limit1 = q + " | STATS count(*)"; | ||
try (EsqlQueryResponse resp = runQuery(limit1, requestIncludeMeta)) { | ||
assertThat(resp.columns().size(), equalTo(1)); | ||
EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); | ||
assertThat(executionInfo.isCrossClusterSearch(), is(false)); | ||
assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); | ||
} | ||
|
||
String limit0 = q + " | LIMIT 0"; | ||
try (EsqlQueryResponse resp = runQuery(limit0, requestIncludeMeta)) { | ||
assertThat(resp.columns().size(), equalTo(1)); | ||
assertThat(getValuesList(resp).size(), equalTo(0)); | ||
EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); | ||
assertThat(executionInfo.isCrossClusterSearch(), is(false)); | ||
assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); | ||
} | ||
} | ||
expectThrows( | ||
VerificationException.class, | ||
containsString("Unknown index [xremote*:events]"), | ||
() -> runQuery("FROM xremote*:events | STATS count(*)", requestIncludeMeta).close() | ||
); | ||
|
||
// try (EsqlQueryResponse resp = runQuery("FROM xremote*:events | STATS count(*)", requestIncludeMeta)) { | ||
// assertThat(resp.columns().size(), equalTo(1)); | ||
// EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); | ||
// assertThat(executionInfo.isCrossClusterSearch(), is(false)); | ||
// assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); | ||
// } | ||
|
||
expectThrows( | ||
VerificationException.class, | ||
containsString("Unknown index [xremote*:events]"), | ||
() -> runQuery("FROM xremote*:events | STATS count(*)", requestIncludeMeta).close() | ||
); | ||
// try (EsqlQueryResponse resp = runQuery("FROM xremote*:events | LIMIT 0", requestIncludeMeta)) { | ||
// assertThat(resp.columns().size(), equalTo(1)); | ||
// assertThat(getValuesList(resp).size(), equalTo(0)); | ||
// EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); | ||
// assertThat(executionInfo.isCrossClusterSearch(), is(false)); | ||
// assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); | ||
// } | ||
} | ||
|
||
public void testCCSWithLimit0() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -383,24 +383,24 @@ protected void testSearchesAgainstNonMatchingIndices(boolean exceptionWithSkipUn | |
// an error is thrown if there is a concrete index that does not match | ||
{ | ||
String q = "FROM nomatch*,cluster-a:nomatch"; | ||
String expectedError = "Unknown index [cluster-a:nomatch,nomatch*]"; | ||
String expectedError = "Unknown index [nomatch*,cluster-a:nomatch]"; | ||
expectVerificationExceptionForQuery(q, expectedError, requestIncludeMeta); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here and in 3 below cases the error message index now matches the original input (notice that the order was different). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this ordering stable - i.e. is it going to always match original order, no matter how many patterns and clusters we have? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct, error uses |
||
} | ||
|
||
// an error is thrown if there are no matching indices at all - local with wildcard, remote with wildcard | ||
{ | ||
String q = "FROM nomatch*,cluster-a:nomatch*"; | ||
String expectedError = "Unknown index [cluster-a:nomatch*,nomatch*]"; | ||
String expectedError = "Unknown index [nomatch*,cluster-a:nomatch*]"; | ||
expectVerificationExceptionForQuery(q, expectedError, requestIncludeMeta); | ||
} | ||
{ | ||
String q = "FROM nomatch,cluster-a:nomatch"; | ||
String expectedError = "Unknown index [cluster-a:nomatch,nomatch]"; | ||
String expectedError = "Unknown index [nomatch,cluster-a:nomatch]"; | ||
expectVerificationExceptionForQuery(q, expectedError, requestIncludeMeta); | ||
} | ||
{ | ||
String q = "FROM nomatch,cluster-a:nomatch*"; | ||
String expectedError = "Unknown index [cluster-a:nomatch*,nomatch]"; | ||
String expectedError = "Unknown index [nomatch,cluster-a:nomatch*]"; | ||
expectVerificationExceptionForQuery(q, expectedError, requestIncludeMeta); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,12 +42,9 @@ | |
import java.util.Collections; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Set; | ||
import java.util.concurrent.atomic.AtomicBoolean; | ||
import java.util.concurrent.atomic.AtomicInteger; | ||
import java.util.concurrent.atomic.AtomicReference; | ||
import java.util.regex.Matcher; | ||
import java.util.regex.Pattern; | ||
import java.util.stream.Collectors; | ||
import java.util.stream.Stream; | ||
|
||
|
@@ -986,16 +983,7 @@ public void testAlias() throws Exception { | |
Request request = esqlRequest("FROM " + index + " | KEEP emp_id | SORT emp_id | LIMIT 100"); | ||
ResponseException error = expectThrows(ResponseException.class, () -> performRequestWithRemoteSearchUser(request)); | ||
assertThat(error.getResponse().getStatusLine().getStatusCode(), equalTo(400)); | ||
String expectedIndexExpressionInError = index.replace("*", "my_remote_cluster"); | ||
Pattern p = Pattern.compile("Unknown index \\[([^\\]]+)\\]"); | ||
Matcher m = p.matcher(error.getMessage()); | ||
assertTrue("Pattern matcher to parse error message did not find matching string: " + error.getMessage(), m.find()); | ||
String unknownIndexExpressionInErrorMessage = m.group(1); | ||
Set<String> actualUnknownIndexes = org.elasticsearch.common.Strings.commaDelimitedListToSet( | ||
unknownIndexExpressionInErrorMessage | ||
); | ||
Set<String> expectedUnknownIndexes = org.elasticsearch.common.Strings.commaDelimitedListToSet(expectedIndexExpressionInError); | ||
assertThat(actualUnknownIndexes, equalTo(expectedUnknownIndexes)); | ||
assertThat(error.getMessage(), containsString("Unknown index [" + index + "]")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Match original query pattern, not semi resolved pattern |
||
} | ||
|
||
for (var index : List.of( | ||
|
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.
We randomly query
foo
or*:foo
. Prior message was actually resolving*
to all known remotes. I do not think this is desired behavior.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.
Okay, we need to keep old resolved
remote_cluster:foo
in case old cluster is coordinating the query.I will check if we can make this check version dependent.