Skip to content
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prior behavior returned empty result for empty index resolution.
Now this is complaining about empty index resolution (consistent with FROM empty-pattern-* case)

// 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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,24 +377,24 @@ public void testSearchesAgainstNonMatchingIndices() throws Exception {
// 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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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).

Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, error uses indexPattern. We are supplying original user input into it now.

}

// 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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
*/
public class PreAnalyzer {

public record PreAnalysis(IndexMode indexMode, IndexPattern index, List<Enrich> enriches, List<IndexPattern> lookupIndices) {
public record PreAnalysis(IndexMode indexMode, IndexPattern indexPattern, List<Enrich> enriches, List<IndexPattern> lookupIndices) {
public static final PreAnalysis EMPTY = new PreAnalysis(null, null, List.of(), List.of());
}

Expand All @@ -37,7 +37,7 @@ public PreAnalysis preAnalyze(LogicalPlan plan) {
protected PreAnalysis doPreAnalyze(LogicalPlan plan) {

Holder<IndexMode> indexMode = new Holder<>();
Holder<IndexPattern> index = new Holder<>();
Holder<IndexPattern> indexPattern = new Holder<>();

List<Enrich> unresolvedEnriches = new ArrayList<>();
List<IndexPattern> lookupIndices = new ArrayList<>();
Expand All @@ -47,7 +47,7 @@ protected PreAnalysis doPreAnalyze(LogicalPlan plan) {
lookupIndices.add(p.indexPattern());
} else if (indexMode.get() == null || indexMode.get() == p.indexMode()) {
indexMode.set(p.indexMode());
index.set(p.indexPattern());
indexPattern.set(p.indexPattern());
} else {
throw new IllegalStateException("index mode is already set");
}
Expand All @@ -58,6 +58,6 @@ protected PreAnalysis doPreAnalyze(LogicalPlan plan) {
// mark plan as preAnalyzed (if it were marked, there would be no analysis)
plan.forEachUp(LogicalPlan::setPreAnalyzed);

return new PreAnalysis(indexMode.get(), index.get(), unresolvedEnriches, lookupIndices);
return new PreAnalysis(indexMode.get(), indexPattern.get(), unresolvedEnriches, lookupIndices);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ public void analyzedPlan(
}

var preAnalysis = preAnalyzer.preAnalyze(parsed);
EsqlCCSUtils.initCrossClusterState(indicesExpressionGrouper, verifier.licenseState(), preAnalysis.index(), executionInfo);
EsqlCCSUtils.initCrossClusterState(indicesExpressionGrouper, verifier.licenseState(), preAnalysis.indexPattern(), executionInfo);

SubscribableListener. //
<EnrichResolution>newForked(l -> enrichPolicyResolver.resolvePolicies(preAnalysis.enriches(), executionInfo, l))
Expand Down Expand Up @@ -653,36 +653,28 @@ private void preAnalyzeMainIndices(
ThreadPool.Names.SEARCH_COORDINATION,
ThreadPool.Names.SYSTEM_READ
);
if (preAnalysis.index() != null) {
String indexExpressionToResolve = EsqlCCSUtils.createIndexExpressionFromAvailableClusters(executionInfo);
if (indexExpressionToResolve.isEmpty()) {
// if this was a pure remote CCS request (no local indices) and all remotes are offline, return an empty IndexResolution
listener.onResponse(
result.withIndexResolution(IndexResolution.valid(new EsIndex(preAnalysis.index().indexPattern(), Map.of(), Map.of())))
);
} else {
boolean includeAllDimensions = false;
// call the EsqlResolveFieldsAction (field-caps) to resolve indices and get field types
if (preAnalysis.indexMode() == IndexMode.TIME_SERIES) {
includeAllDimensions = true;
// TODO: Maybe if no indices are returned, retry without index mode and provide a clearer error message.
var indexModeFilter = new TermQueryBuilder(IndexModeFieldMapper.NAME, IndexMode.TIME_SERIES.getName());
if (requestFilter != null) {
requestFilter = new BoolQueryBuilder().filter(requestFilter).filter(indexModeFilter);
} else {
requestFilter = indexModeFilter;
}
if (preAnalysis.indexPattern() != null) {
boolean includeAllDimensions = false;
// call the EsqlResolveFieldsAction (field-caps) to resolve indices and get field types
if (preAnalysis.indexMode() == IndexMode.TIME_SERIES) {
includeAllDimensions = true;
// TODO: Maybe if no indices are returned, retry without index mode and provide a clearer error message.
var indexModeFilter = new TermQueryBuilder(IndexModeFieldMapper.NAME, IndexMode.TIME_SERIES.getName());
if (requestFilter != null) {
requestFilter = new BoolQueryBuilder().filter(requestFilter).filter(indexModeFilter);
} else {
requestFilter = indexModeFilter;
}
indexResolver.resolveAsMergedMapping(
indexExpressionToResolve,
result.fieldNames,
requestFilter,
includeAllDimensions,
listener.delegateFailure((l, indexResolution) -> {
l.onResponse(result.withIndexResolution(indexResolution));
})
);
}
indexResolver.resolveAsMergedMapping(
preAnalysis.indexPattern().indexPattern(),
result.fieldNames,
requestFilter,
includeAllDimensions,
listener.delegateFailure((l, indexResolution) -> {
l.onResponse(result.withIndexResolution(indexResolution));
})
);
} else {
// occurs when dealing with local relations (row a = 1)
listener.onResponse(result.withIndexResolution(IndexResolution.invalid("[none specified]")));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ private static FieldCapabilitiesRequest createFieldCapsRequest(
req.fields(fieldNames.toArray(String[]::new));
req.includeUnmapped(true);
req.indexFilter(requestFilter);
req.returnLocalAll(false);
// lenient because we throw our own errors looking at the response e.g. if something was not resolved
// also because this way security doesn't throw authorization exceptions but rather honors ignore_unavailable
req.indicesOptions(FIELD_CAPS_INDICES_OPTIONS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -530,12 +530,12 @@ private LogicalPlan analyzedPlan(LogicalPlan parsed, CsvTestsDataLoader.MultiInd

private static CsvTestsDataLoader.MultiIndexTestDataset testDatasets(LogicalPlan parsed) {
var preAnalysis = new PreAnalyzer().preAnalyze(parsed);
if (preAnalysis.index() == null) {
if (preAnalysis.indexPattern() == null) {
// If the data set doesn't matter we'll just grab one we know works. Employees is fine.
return CsvTestsDataLoader.MultiIndexTestDataset.of(CSV_DATASET_MAP.get("employees"));
}

String indexName = preAnalysis.index().indexPattern();
String indexName = preAnalysis.indexPattern().indexPattern();
List<CsvTestsDataLoader.TestDataset> datasets = new ArrayList<>();
if (indexName.endsWith("*")) {
String indexPrefix = indexName.substring(0, indexName.length() - 1);
Expand Down