Skip to content

Commit 522234a

Browse files
smalyshevsmalyshev
committed
More fixes
1 parent 215eba2 commit 522234a

File tree

4 files changed

+76
-19
lines changed

4 files changed

+76
-19
lines changed

‎x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterLookupJoinIT.java

Lines changed: 51 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest;
1111
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequestBuilder;
1212
import org.elasticsearch.client.internal.Client;
13+
import org.elasticsearch.common.settings.Settings;
1314
import org.elasticsearch.xpack.core.enrich.EnrichPolicy;
1415
import org.elasticsearch.xpack.core.enrich.action.ExecuteEnrichPolicyAction;
1516
import org.elasticsearch.xpack.core.enrich.action.PutEnrichPolicyAction;
@@ -374,7 +375,7 @@ public void testLookupJoinMissingKey() throws IOException {
374375
);
375376
assertThat(ex.getMessage(), containsString("Unknown column [local_tag] in right side of join"));
376377

377-
// Add KEEP clause to try and trick the field-caps result parser
378+
// Add KEEP clause to try and trick the field-caps result parser into returning empty mapping
378379
ex = expectThrows(
379380
VerificationException.class,
380381
() -> runQuery("FROM logs-* | LOOKUP JOIN values_lookup ON v | KEEP v", randomBoolean())
@@ -385,22 +386,13 @@ public void testLookupJoinMissingKey() throws IOException {
385386
VerificationException.class,
386387
() -> runQuery("FROM logs-*,c*:logs-* | LOOKUP JOIN values_lookup ON v | KEEP v", randomBoolean())
387388
);
388-
// FIXME: strictly speaking this message is not correct, as the index is available, but the field is not
389-
assertThat(ex.getMessage(), containsString("lookup index [values_lookup] is not available"));
390-
391-
try (EsqlQueryResponse resp = runQuery("FROM c*:logs-* | LOOKUP JOIN values_lookup ON v | KEEP v", randomBoolean())) {
392-
List<List<Object>> values = getValuesList(resp);
393-
assertThat(values, hasSize(0));
394-
EsqlExecutionInfo executionInfo = resp.getExecutionInfo();
395-
assertThat(executionInfo.getClusters().size(), equalTo(1));
396-
assertTrue(executionInfo.isPartial());
389+
assertThat(ex.getMessage(), containsString("Unknown column [v] in right side of join"));
397390

398-
var remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1);
399-
assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED));
400-
assertThat(remoteCluster.getFailures().size(), equalTo(1));
401-
var failure = remoteCluster.getFailures().get(0);
402-
assertThat(failure.reason(), containsString("lookup index [values_lookup] is not available in remote cluster [cluster-a]"));
403-
}
391+
ex = expectThrows(
392+
VerificationException.class,
393+
() -> runQuery("FROM c*:logs-* | LOOKUP JOIN values_lookup ON v | KEEP v", randomBoolean())
394+
);
395+
assertThat(ex.getMessage(), containsString("Unknown column [v] in right side of join"));
404396

405397
setSkipUnavailable(REMOTE_CLUSTER_1, false);
406398
try (
@@ -422,19 +414,40 @@ public void testLookupJoinMissingKey() throws IOException {
422414
assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL));
423415
}
424416

425-
// Add KEEP clause to try and trick the field-caps result parser
417+
// Add KEEP clause to try and trick the field-caps result parser into returning empty mapping
426418
ex = expectThrows(
427419
VerificationException.class,
428420
() -> runQuery("FROM c*:logs-* | LOOKUP JOIN values_lookup ON v | KEEP v", randomBoolean())
429421
);
430-
assertThat(ex.getMessage(), containsString("lookup index [values_lookup] is not available in remote cluster [cluster-a]"));
422+
assertThat(ex.getMessage(), containsString("Unknown column [v] in right side of join"));
431423

432424
ex = expectThrows(
433425
VerificationException.class,
434426
() -> runQuery("FROM logs-*,c*:logs-* | LOOKUP JOIN values_lookup ON v | KEEP v", randomBoolean())
435427
);
436-
assertThat(ex.getMessage(), containsString("lookup index [values_lookup] is not available in remote cluster [cluster-a]"));
428+
assertThat(ex.getMessage(), containsString("Unknown column [v] in right side of join"));
429+
}
430+
431+
public void testLookupJoinEmptyIndex() throws IOException {
432+
setupClusters(2);
433+
populateEmptyIndices(LOCAL_CLUSTER, "values_lookup");
434+
populateEmptyIndices(REMOTE_CLUSTER_1, "values_lookup");
435+
436+
setSkipUnavailable(REMOTE_CLUSTER_1, false);
437437

438+
Exception ex;
439+
for (String index : List.of("values_lookup", "values_lookup_map", "values_lookup_map_lookup")) {
440+
ex = expectThrows(
441+
VerificationException.class,
442+
() -> runQuery("FROM logs-* | LOOKUP JOIN " + index + " ON v | KEEP v", randomBoolean())
443+
);
444+
assertThat(ex.getMessage(), containsString("Unknown column [v] in right side of join"));
445+
ex = expectThrows(
446+
VerificationException.class,
447+
() -> runQuery("FROM c*:logs-* | LOOKUP JOIN " + index + " ON v | KEEP v", randomBoolean())
448+
);
449+
assertThat(ex.getMessage(), containsString("Unknown column [v] in right side of join"));
450+
}
438451
}
439452

440453
public void testLookupJoinIndexMode() throws IOException {
@@ -570,4 +583,23 @@ protected void setupAlias(String clusterAlias, String indexName, String aliasNam
570583
assertAcked(client.admin().indices().aliases(indicesAliasesRequestBuilder.request()));
571584
}
572585

586+
protected void populateEmptyIndices(String clusterAlias, String indexName) {
587+
Client client = client(clusterAlias);
588+
// Empty body
589+
assertAcked(client.admin().indices().prepareCreate(indexName));
590+
client.admin().indices().prepareRefresh(indexName).get();
591+
// mappings + settings
592+
assertAcked(
593+
client.admin()
594+
.indices()
595+
.prepareCreate(indexName + "_map_lookup")
596+
.setMapping()
597+
.setSettings(Settings.builder().put("index.mode", "lookup"))
598+
);
599+
client.admin().indices().prepareRefresh(indexName + "_map_lookup").get();
600+
// mappings only
601+
assertAcked(client.admin().indices().prepareCreate(indexName + "_map").setMapping());
602+
client.admin().indices().prepareRefresh(indexName + "_map").get();
603+
}
604+
573605
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,16 @@ private PreAnalysisResult receiveLookupIndexResolution(
528528
}
529529
return result.addLookupIndexResolution(index, lookupIndexResolution);
530530
}
531+
532+
if (lookupIndexResolution.get().indexNameWithModes().isEmpty() && lookupIndexResolution.resolvedIndices().isEmpty() == false) {
533+
// This is a weird situation - we have empty index list but non-empty resolution. This is likely because IndexResolver
534+
// got an empty map and pretends to have an empty resolution. This means this query will fail, since lookup fields will not
535+
// match, but here we can pretend it's ok to pass it on to the verifier and generate a correct error message.
536+
// Note this only happens if the map is completely empty, which means it's going to error out anyway, since we should have
537+
// at least the key field there.
538+
return result.addLookupIndexResolution(index, lookupIndexResolution);
539+
}
540+
531541
// Collect resolved clusters from the index resolution, verify that each cluster has a single resolution for the lookup index
532542
Map<String, String> clustersWithResolvedIndices = new HashMap<>(lookupIndexResolution.resolvedIndices().size());
533543
lookupIndexResolution.get().indexNameWithModes().forEach((indexName, indexMode) -> {

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/IndexResolver.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,11 @@ public static IndexResolution mergedMappings(String indexPattern, FieldCapabilit
157157
allEmpty &= ir.get().isEmpty();
158158
}
159159
// If all the mappings are empty we return an empty set of resolved indices to line up with QL
160+
// Introduced with #46775
161+
// We need to be able to differentiate between an empty mapping index and an empty index due to fields not being found. An empty
162+
// mapping index will generate no columns (important) for a query like FROM empty-mapping-index, whereas an empty result here but
163+
// for fields that do not exist in the index (but the index has a mapping) will result in "VerificationException Unknown column"
164+
// errors.
160165
var index = new EsIndex(indexPattern, rootFields, allEmpty ? Map.of() : concreteIndices, partiallyUnmappedFields);
161166
var failures = EsqlCCSUtils.groupFailuresPerCluster(fieldCapsResponse.getFailures());
162167
return IndexResolution.valid(index, concreteIndices.keySet(), failures);

‎x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/190_lookup_join.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,3 +230,13 @@ lookup-no-key:
230230

231231
- match: { error.type: "verification_exception" }
232232
- contains: { error.reason: "Unknown column [key] in right side of join" }
233+
---
234+
lookup-no-key-only-key:
235+
- do:
236+
esql.query:
237+
body:
238+
query: 'FROM test | LOOKUP JOIN test-lookup-no-key ON key | KEEP key'
239+
catch: "bad_request"
240+
241+
- match: { error.type: "verification_exception" }
242+
- contains: { error.reason: "Unknown column [key] in right side of join" }

0 commit comments

Comments
 (0)