Skip to content

Commit f4242d3

Browse files
committed
ESQL: Fix wrong marking of a field as unmapped when indices shared the same mapping (#133298)
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 653f590 commit f4242d3

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
@@ -92,13 +92,15 @@ public void resolveAsMergedMapping(
9292

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

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

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

133-
List<IndexFieldCapabilities> fcs = fieldsCaps.get(fullName);
135+
var fieldCap = fieldsCaps.get(fullName);
136+
List<IndexFieldCapabilities> fcs = fieldCap.fieldCapabilities;
134137
EsField field = firstUnsupportedParent == null
135138
? createField(fieldCapsResponse, name, fullName, fcs, isAlias)
136139
: new UnsupportedEsField(
@@ -140,8 +143,7 @@ public static IndexResolution mergedMappings(String indexPattern, FieldCapabilit
140143
new HashMap<>()
141144
);
142145
fields.put(name, field);
143-
144-
var isPartiallyUnmapped = fcs.size() < numberOfIndices;
146+
var isPartiallyUnmapped = fcs.size() + indexMappingHashDuplicates.getOrDefault(fieldCap.indexMappingHash, 0) < numberOfIndices;
145147
if (isPartiallyUnmapped) {
146148
partiallyUnmappedFields.add(fullName);
147149
}
@@ -162,23 +164,46 @@ public static IndexResolution mergedMappings(String indexPattern, FieldCapabilit
162164
return IndexResolution.valid(index, concreteIndices.keySet(), failures);
163165
}
164166

165-
private static Map<String, List<IndexFieldCapabilities>> collectFieldCaps(FieldCapabilitiesResponse fieldCapsResponse) {
166-
Set<String> seenHashes = new HashSet<>();
167-
Map<String, List<IndexFieldCapabilities>> fieldsCaps = new HashMap<>();
167+
private record IndexFieldCapabilitiesWithSourceHash(List<IndexFieldCapabilities> fieldCapabilities, String indexMappingHash) {}
168+
169+
private record CollectedFieldCaps(
170+
Map<String, IndexFieldCapabilitiesWithSourceHash> fieldsCaps,
171+
// The map won't contain entries without duplicates, i.e., it's number of occurrences - 1.
172+
Map<String, Integer> indexMappingHashDuplicates
173+
) {}
174+
175+
private static CollectedFieldCaps collectFieldCaps(FieldCapabilitiesResponse fieldCapsResponse) {
176+
Map<String, Integer> indexMappingHashToDuplicateCount = new HashMap<>();
177+
Map<String, IndexFieldCapabilitiesWithSourceHash> fieldsCaps = new HashMap<>();
178+
168179
for (FieldCapabilitiesIndexResponse response : fieldCapsResponse.getIndexResponses()) {
169-
if (seenHashes.add(response.getIndexMappingHash()) == false) {
180+
if (indexMappingHashToDuplicateCount.compute(response.getIndexMappingHash(), (k, v) -> v == null ? 1 : v + 1) > 1) {
170181
continue;
171182
}
172183
for (IndexFieldCapabilities fc : response.get().values()) {
173184
if (fc.isMetadatafield()) {
174185
// ESQL builds the metadata fields if they are asked for without using the resolution.
175186
continue;
176187
}
177-
List<IndexFieldCapabilities> all = fieldsCaps.computeIfAbsent(fc.name(), (_key) -> new ArrayList<>());
188+
List<IndexFieldCapabilities> all = fieldsCaps.computeIfAbsent(
189+
fc.name(),
190+
(_key) -> new IndexFieldCapabilitiesWithSourceHash(new ArrayList<>(), response.getIndexMappingHash())
191+
).fieldCapabilities;
178192
all.add(fc);
179193
}
180194
}
181-
return fieldsCaps;
195+
196+
var iterator = indexMappingHashToDuplicateCount.entrySet().iterator();
197+
while (iterator.hasNext()) {
198+
var next = iterator.next();
199+
if (next.getValue() <= 1) {
200+
iterator.remove();
201+
} else {
202+
next.setValue(next.getValue() - 1);
203+
}
204+
}
205+
206+
return new CollectedFieldCaps(fieldsCaps, indexMappingHashToDuplicateCount);
182207
}
183208

184209
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
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse;
1313
import org.elasticsearch.action.fieldcaps.IndexFieldCapabilities;
1414
import org.elasticsearch.action.fieldcaps.IndexFieldCapabilitiesBuilder;
15+
import org.elasticsearch.common.hash.MessageDigests;
1516
import org.elasticsearch.common.lucene.BytesRefs;
1617
import org.elasticsearch.common.settings.Settings;
1718
import org.elasticsearch.index.IndexMode;
@@ -89,6 +90,7 @@
8990
import org.elasticsearch.xpack.esql.session.IndexResolver;
9091

9192
import java.io.IOException;
93+
import java.nio.charset.StandardCharsets;
9294
import java.time.Period;
9395
import java.util.ArrayList;
9496
import java.util.List;
@@ -3017,6 +3019,27 @@ public void testResolveInsist_multiIndexFieldPartiallyExistsWithMultiTypesNoKeyw
30173019
assertThat(attr.unresolvedMessage(), is(expected));
30183020
}
30193021

3022+
public void testResolveInsist_multiIndexSameMapping_fieldIsMapped() {
3023+
assumeTrue("Requires UNMAPPED FIELDS", EsqlCapabilities.Cap.UNMAPPED_FIELDS.isEnabled());
3024+
3025+
IndexResolution resolution = IndexResolver.mergedMappings(
3026+
"foo, bar",
3027+
new FieldCapabilitiesResponse(
3028+
List.of(
3029+
fieldCapabilitiesIndexResponse("foo", messageResponseMap("long")),
3030+
fieldCapabilitiesIndexResponse("bar", messageResponseMap("long"))
3031+
),
3032+
List.of()
3033+
)
3034+
);
3035+
var plan = analyze("FROM foo, bar | INSIST_🐔 message", analyzer(resolution, TEST_VERIFIER));
3036+
var limit = as(plan, Limit.class);
3037+
var insist = as(limit.child(), Insist.class);
3038+
var attribute = (FieldAttribute) EsqlTestUtils.singleValue(insist.output());
3039+
assertThat(attribute.name(), is("message"));
3040+
assertThat(attribute.dataType(), is(DataType.LONG));
3041+
}
3042+
30203043
public void testResolveInsist_multiIndexFieldPartiallyExistsWithMultiTypesWithKeyword_createsAnInvalidMappedField() {
30213044
assumeTrue("Requires UNMAPPED FIELDS", EsqlCapabilities.Cap.UNMAPPED_FIELDS.isEnabled());
30223045

@@ -3472,7 +3495,11 @@ private static FieldCapabilitiesIndexResponse fieldCapabilitiesIndexResponse(
34723495
String indexName,
34733496
Map<String, IndexFieldCapabilities> fields
34743497
) {
3475-
return new FieldCapabilitiesIndexResponse(indexName, indexName, fields, false, IndexMode.STANDARD);
3498+
String indexMappingHash = new String(
3499+
MessageDigests.sha256().digest(fields.toString().getBytes(StandardCharsets.UTF_8)),
3500+
StandardCharsets.UTF_8
3501+
);
3502+
return new FieldCapabilitiesIndexResponse(indexName, indexMappingHash, fields, false, IndexMode.STANDARD);
34763503
}
34773504

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

0 commit comments

Comments
 (0)