Skip to content

Commit 6e4239e

Browse files
authored
ESQL: Fix wrong marking of a field as unmapped when indices shared the same mapping (elastic#133298) (elastic#133698)
A follow up fixing https://github.com/elastic/elasticsearch/pull/119886/files/e32a4463e8a4015c7be17cb984a80e79ed1e45c5#r2287545802. The original issue, as mentioned in the comment, was triggered by multiple indices sharing the same mapping. This PR fixes it by reading the FieldCapabilitiesResponse directly instead.
1 parent 540e317 commit 6e4239e

File tree

2 files changed

+64
-12
lines changed

2 files changed

+64
-12
lines changed

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

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,15 @@ public void resolveAsMergedMapping(
9393

9494
// public for testing only
9595
public static IndexResolution mergedMappings(String indexPattern, FieldCapabilitiesResponse fieldCapsResponse) {
96-
var numberOfIndices = fieldCapsResponse.getIndexResponses().size();
9796
assert ThreadPool.assertCurrentThreadPool(ThreadPool.Names.SEARCH_COORDINATION); // too expensive to run this on a transport worker
97+
var numberOfIndices = fieldCapsResponse.getIndexResponses().size();
9898
if (fieldCapsResponse.getIndexResponses().isEmpty()) {
9999
return IndexResolution.notFound(indexPattern);
100100
}
101101

102-
Map<String, List<IndexFieldCapabilities>> fieldsCaps = collectFieldCaps(fieldCapsResponse);
102+
var collectedFieldCaps = collectFieldCaps(fieldCapsResponse);
103+
Map<String, IndexFieldCapabilitiesWithSourceHash> fieldsCaps = collectedFieldCaps.fieldsCaps;
104+
Map<String, Integer> indexMappingHashDuplicates = collectedFieldCaps.indexMappingHashDuplicates;
103105

104106
// Build hierarchical fields - it's easier to do it in sorted order so the object fields come first.
105107
// TODO flattened is simpler - could we get away with that?
@@ -131,7 +133,8 @@ public static IndexResolution mergedMappings(String indexPattern, FieldCapabilit
131133
}
132134
// TODO we're careful to make isAlias match IndexResolver - but do we use it?
133135

134-
List<IndexFieldCapabilities> fcs = fieldsCaps.get(fullName);
136+
var fieldCap = fieldsCaps.get(fullName);
137+
List<IndexFieldCapabilities> fcs = fieldCap.fieldCapabilities;
135138
EsField field = firstUnsupportedParent == null
136139
? createField(fieldCapsResponse, name, fullName, fcs, isAlias)
137140
: new UnsupportedEsField(
@@ -141,8 +144,7 @@ public static IndexResolution mergedMappings(String indexPattern, FieldCapabilit
141144
new HashMap<>()
142145
);
143146
fields.put(name, field);
144-
145-
var isPartiallyUnmapped = fcs.size() < numberOfIndices;
147+
var isPartiallyUnmapped = fcs.size() + indexMappingHashDuplicates.getOrDefault(fieldCap.indexMappingHash, 0) < numberOfIndices;
146148
if (isPartiallyUnmapped) {
147149
partiallyUnmappedFields.add(fullName);
148150
}
@@ -166,23 +168,46 @@ public static IndexResolution mergedMappings(String indexPattern, FieldCapabilit
166168
return IndexResolution.valid(index, concreteIndices.keySet(), unavailableRemotes);
167169
}
168170

169-
private static Map<String, List<IndexFieldCapabilities>> collectFieldCaps(FieldCapabilitiesResponse fieldCapsResponse) {
170-
Set<String> seenHashes = new HashSet<>();
171-
Map<String, List<IndexFieldCapabilities>> fieldsCaps = new HashMap<>();
171+
private record IndexFieldCapabilitiesWithSourceHash(List<IndexFieldCapabilities> fieldCapabilities, String indexMappingHash) {}
172+
173+
private record CollectedFieldCaps(
174+
Map<String, IndexFieldCapabilitiesWithSourceHash> fieldsCaps,
175+
// The map won't contain entries without duplicates, i.e., it's number of occurrences - 1.
176+
Map<String, Integer> indexMappingHashDuplicates
177+
) {}
178+
179+
private static CollectedFieldCaps collectFieldCaps(FieldCapabilitiesResponse fieldCapsResponse) {
180+
Map<String, Integer> indexMappingHashToDuplicateCount = new HashMap<>();
181+
Map<String, IndexFieldCapabilitiesWithSourceHash> fieldsCaps = new HashMap<>();
182+
172183
for (FieldCapabilitiesIndexResponse response : fieldCapsResponse.getIndexResponses()) {
173-
if (seenHashes.add(response.getIndexMappingHash()) == false) {
184+
if (indexMappingHashToDuplicateCount.compute(response.getIndexMappingHash(), (k, v) -> v == null ? 1 : v + 1) > 1) {
174185
continue;
175186
}
176187
for (IndexFieldCapabilities fc : response.get().values()) {
177188
if (fc.isMetadatafield()) {
178189
// ESQL builds the metadata fields if they are asked for without using the resolution.
179190
continue;
180191
}
181-
List<IndexFieldCapabilities> all = fieldsCaps.computeIfAbsent(fc.name(), (_key) -> new ArrayList<>());
192+
List<IndexFieldCapabilities> all = fieldsCaps.computeIfAbsent(
193+
fc.name(),
194+
(_key) -> new IndexFieldCapabilitiesWithSourceHash(new ArrayList<>(), response.getIndexMappingHash())
195+
).fieldCapabilities;
182196
all.add(fc);
183197
}
184198
}
185-
return fieldsCaps;
199+
200+
var iterator = indexMappingHashToDuplicateCount.entrySet().iterator();
201+
while (iterator.hasNext()) {
202+
var next = iterator.next();
203+
if (next.getValue() <= 1) {
204+
iterator.remove();
205+
} else {
206+
next.setValue(next.getValue() - 1);
207+
}
208+
}
209+
210+
return new CollectedFieldCaps(fieldsCaps, indexMappingHashToDuplicateCount);
186211
}
187212

188213
private static EsField createField(

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesIndexResponse;
1212
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse;
1313
import org.elasticsearch.action.fieldcaps.IndexFieldCapabilities;
14+
import org.elasticsearch.common.hash.MessageDigests;
1415
import org.elasticsearch.common.settings.Settings;
1516
import org.elasticsearch.index.IndexMode;
1617
import org.elasticsearch.index.analysis.IndexAnalyzers;
@@ -64,6 +65,7 @@
6465
import org.elasticsearch.xpack.esql.session.IndexResolver;
6566

6667
import java.io.IOException;
68+
import java.nio.charset.StandardCharsets;
6769
import java.util.ArrayList;
6870
import java.util.List;
6971
import java.util.Map;
@@ -2743,6 +2745,27 @@ public void testResolveInsist_multiIndexFieldPartiallyExistsWithMultiTypesNoKeyw
27432745
assertThat(attr.unresolvedMessage(), is(expected));
27442746
}
27452747

2748+
public void testResolveInsist_multiIndexSameMapping_fieldIsMapped() {
2749+
assumeTrue("Requires UNMAPPED FIELDS", EsqlCapabilities.Cap.UNMAPPED_FIELDS.isEnabled());
2750+
2751+
IndexResolution resolution = IndexResolver.mergedMappings(
2752+
"foo, bar",
2753+
new FieldCapabilitiesResponse(
2754+
List.of(
2755+
fieldCapabilitiesIndexResponse("foo", messageResponseMap("long")),
2756+
fieldCapabilitiesIndexResponse("bar", messageResponseMap("long"))
2757+
),
2758+
List.of()
2759+
)
2760+
);
2761+
var plan = analyze("FROM foo, bar | INSIST_🐔 message", analyzer(resolution, TEST_VERIFIER));
2762+
var limit = as(plan, Limit.class);
2763+
var insist = as(limit.child(), Insist.class);
2764+
var attribute = (FieldAttribute) EsqlTestUtils.singleValue(insist.output());
2765+
assertThat(attribute.name(), is("message"));
2766+
assertThat(attribute.dataType(), is(DataType.LONG));
2767+
}
2768+
27462769
public void testResolveInsist_multiIndexFieldPartiallyExistsWithMultiTypesWithKeyword_createsAnInvalidMappedField() {
27472770
assumeTrue("Requires UNMAPPED FIELDS", EsqlCapabilities.Cap.UNMAPPED_FIELDS.isEnabled());
27482771

@@ -2798,7 +2821,11 @@ private static FieldCapabilitiesIndexResponse fieldCapabilitiesIndexResponse(
27982821
String indexName,
27992822
Map<String, IndexFieldCapabilities> fields
28002823
) {
2801-
return new FieldCapabilitiesIndexResponse(indexName, indexName, fields, false, IndexMode.STANDARD);
2824+
String indexMappingHash = new String(
2825+
MessageDigests.sha256().digest(fields.toString().getBytes(StandardCharsets.UTF_8)),
2826+
StandardCharsets.UTF_8
2827+
);
2828+
return new FieldCapabilitiesIndexResponse(indexName, indexMappingHash, fields, false, IndexMode.STANDARD);
28022829
}
28032830

28042831
private static Map<String, IndexFieldCapabilities> messageResponseMap(String date) {

0 commit comments

Comments
 (0)