Skip to content

Commit 165c70e

Browse files
authored
Fix index lookup when field-caps returns empty mapping (#132138)
* Fix index lookup when field-caps returns empty mapping
1 parent 89ad24f commit 165c70e

File tree

5 files changed

+106
-4
lines changed

5 files changed

+106
-4
lines changed

docs/changelog/132138.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 132138
2+
summary: Fix lookup index resolution when field-caps returns empty mapping
3+
area: ES|QL
4+
type: bug
5+
issues:
6+
- 132105

x-pack/plugin/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ tasks.named("yamlRestCompatTestTransform").configure({ task ->
139139
task.skipTest("esql/192_lookup_join_on_aliases/alias-pattern-multiple", "Error message changed")
140140
task.skipTest("esql/192_lookup_join_on_aliases/fails when alias or pattern resolves to multiple", "Error message changed")
141141
task.skipTest("esql/10_basic/Test wrong LIMIT parameter", "Error message changed")
142+
task.skipTest("esql/190_lookup_join/lookup-no-key-only-key", "Requires the fix")
142143
})
143144

144145
tasks.named('yamlRestCompatTest').configure {

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

Lines changed: 79 additions & 4 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;
@@ -289,6 +290,9 @@ public void testLookupJoinMissingKey() throws IOException {
289290
populateLookupIndex(REMOTE_CLUSTER_1, "values_lookup", 10);
290291

291292
setSkipUnavailable(REMOTE_CLUSTER_1, true);
293+
294+
Exception ex;
295+
292296
try (
293297
// Using local_tag as key which is not present in remote index
294298
EsqlQueryResponse resp = runQuery(
@@ -362,10 +366,7 @@ public void testLookupJoinMissingKey() throws IOException {
362366
}
363367

364368
// TODO: verify whether this should be an error or not when the key field is missing
365-
Exception ex = expectThrows(
366-
VerificationException.class,
367-
() -> runQuery("FROM c*:logs-* | LOOKUP JOIN values_lookup ON v", randomBoolean())
368-
);
369+
ex = expectThrows(VerificationException.class, () -> runQuery("FROM c*:logs-* | LOOKUP JOIN values_lookup ON v", randomBoolean()));
369370
assertThat(ex.getMessage(), containsString("Unknown column [v] in right side of join"));
370371

371372
ex = expectThrows(
@@ -374,6 +375,25 @@ public void testLookupJoinMissingKey() throws IOException {
374375
);
375376
assertThat(ex.getMessage(), containsString("Unknown column [local_tag] in right side of join"));
376377

378+
// Add KEEP clause to try and trick the field-caps result parser into returning empty mapping
379+
ex = expectThrows(
380+
VerificationException.class,
381+
() -> runQuery("FROM logs-* | LOOKUP JOIN values_lookup ON v | KEEP v", randomBoolean())
382+
);
383+
assertThat(ex.getMessage(), containsString("Unknown column [v] in right side of join"));
384+
385+
ex = expectThrows(
386+
VerificationException.class,
387+
() -> runQuery("FROM logs-*,c*:logs-* | LOOKUP JOIN values_lookup ON v | KEEP v", randomBoolean())
388+
);
389+
assertThat(ex.getMessage(), containsString("Unknown column [v] in right side of join"));
390+
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"));
396+
377397
setSkipUnavailable(REMOTE_CLUSTER_1, false);
378398
try (
379399
// Using local_tag as key which is not present in remote index
@@ -393,6 +413,42 @@ public void testLookupJoinMissingKey() throws IOException {
393413
// FIXME: verify whether we need to succeed or fail here
394414
assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL));
395415
}
416+
417+
// Add KEEP clause to try and trick the field-caps result parser into returning empty mapping
418+
ex = expectThrows(
419+
VerificationException.class,
420+
() -> runQuery("FROM c*:logs-* | LOOKUP JOIN values_lookup ON v | KEEP v", randomBoolean())
421+
);
422+
assertThat(ex.getMessage(), containsString("Unknown column [v] in right side of join"));
423+
424+
ex = expectThrows(
425+
VerificationException.class,
426+
() -> runQuery("FROM logs-*,c*:logs-* | LOOKUP JOIN values_lookup ON v | KEEP v", randomBoolean())
427+
);
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+
// Should work the same with both settings
437+
setSkipUnavailable(REMOTE_CLUSTER_1, randomBoolean());
438+
439+
Exception ex;
440+
for (String index : List.of("values_lookup", "values_lookup_map", "values_lookup_map_lookup")) {
441+
ex = expectThrows(
442+
VerificationException.class,
443+
() -> runQuery("FROM logs-* | LOOKUP JOIN " + index + " ON v | KEEP v", randomBoolean())
444+
);
445+
assertThat(ex.getMessage(), containsString("Unknown column [v] in right side of join"));
446+
ex = expectThrows(
447+
VerificationException.class,
448+
() -> runQuery("FROM c*:logs-* | LOOKUP JOIN " + index + " ON v | KEEP v", randomBoolean())
449+
);
450+
assertThat(ex.getMessage(), containsString("Unknown column [v] in right side of join"));
451+
}
396452
}
397453

398454
public void testLookupJoinIndexMode() throws IOException {
@@ -528,4 +584,23 @@ protected void setupAlias(String clusterAlias, String indexName, String aliasNam
528584
assertAcked(client.admin().indices().aliases(indicesAliasesRequestBuilder.request()));
529585
}
530586

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

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,11 @@ private PreAnalysisResult receiveLookupIndexResolution(
497497
}
498498
if (executionInfo.getClusters().isEmpty() || executionInfo.isCrossClusterSearch() == false) {
499499
// Local only case, still do some checks, since we moved analysis checks here
500+
if (lookupIndexResolution.get().indexNameWithModes().isEmpty()) {
501+
// This is not OK, but we proceed with it as we do with invalid resolution, and it will fail on the verification
502+
// because lookup field will be missing.
503+
return result.addLookupIndexResolution(index, lookupIndexResolution);
504+
}
500505
if (lookupIndexResolution.get().indexNameWithModes().size() > 1) {
501506
throw new VerificationException(
502507
"Lookup Join requires a single lookup mode index; [" + index + "] resolves to multiple indices"
@@ -516,6 +521,16 @@ private PreAnalysisResult receiveLookupIndexResolution(
516521
}
517522
return result.addLookupIndexResolution(index, lookupIndexResolution);
518523
}
524+
525+
if (lookupIndexResolution.get().indexNameWithModes().isEmpty() && lookupIndexResolution.resolvedIndices().isEmpty() == false) {
526+
// This is a weird situation - we have empty index list but non-empty resolution. This is likely because IndexResolver
527+
// got an empty map and pretends to have an empty resolution. This means this query will fail, since lookup fields will not
528+
// match, but here we can pretend it's ok to pass it on to the verifier and generate a correct error message.
529+
// Note this only happens if the map is completely empty, which means it's going to error out anyway, since we should have
530+
// at least the key field there.
531+
return result.addLookupIndexResolution(index, lookupIndexResolution);
532+
}
533+
519534
// Collect resolved clusters from the index resolution, verify that each cluster has a single resolution for the lookup index
520535
Map<String, String> clustersWithResolvedIndices = new HashMap<>(lookupIndexResolution.resolvedIndices().size());
521536
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);

0 commit comments

Comments
 (0)