Skip to content

Commit 19c035f

Browse files
authored
ESQL: Fix wrong marking of a field as unmapped when indices shared the same mapping (elastic#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 072d5fd commit 19c035f

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,14 +93,16 @@ 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

102102
// For each field name, store a list of the field caps responses from each index
103-
Map<String, List<IndexFieldCapabilities>> fieldsCaps = collectFieldCaps(fieldCapsResponse);
103+
var collectedFieldCaps = collectFieldCaps(fieldCapsResponse);
104+
Map<String, IndexFieldCapabilitiesWithSourceHash> fieldsCaps = collectedFieldCaps.fieldsCaps;
105+
Map<String, Integer> indexMappingHashDuplicates = collectedFieldCaps.indexMappingHashDuplicates;
104106

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

136-
List<IndexFieldCapabilities> fcs = fieldsCaps.get(fullName);
138+
var fieldCap = fieldsCaps.get(fullName);
139+
List<IndexFieldCapabilities> fcs = fieldCap.fieldCapabilities;
137140
EsField field = firstUnsupportedParent == null
138141
? createField(fieldCapsResponse, name, fullName, fcs, isAlias)
139142
: new UnsupportedEsField(
@@ -143,8 +146,7 @@ public static IndexResolution mergedMappings(String indexPattern, FieldCapabilit
143146
new HashMap<>()
144147
);
145148
fields.put(name, field);
146-
147-
var isPartiallyUnmapped = fcs.size() < numberOfIndices;
149+
var isPartiallyUnmapped = fcs.size() + indexMappingHashDuplicates.getOrDefault(fieldCap.indexMappingHash, 0) < numberOfIndices;
148150
if (isPartiallyUnmapped) {
149151
partiallyUnmappedFields.add(fullName);
150152
}
@@ -170,23 +172,46 @@ public static IndexResolution mergedMappings(String indexPattern, FieldCapabilit
170172
return IndexResolution.valid(index, concreteIndices.keySet(), failures);
171173
}
172174

173-
private static Map<String, List<IndexFieldCapabilities>> collectFieldCaps(FieldCapabilitiesResponse fieldCapsResponse) {
174-
Set<String> seenHashes = new HashSet<>();
175-
Map<String, List<IndexFieldCapabilities>> fieldsCaps = new HashMap<>();
175+
private record IndexFieldCapabilitiesWithSourceHash(List<IndexFieldCapabilities> fieldCapabilities, String indexMappingHash) {}
176+
177+
private record CollectedFieldCaps(
178+
Map<String, IndexFieldCapabilitiesWithSourceHash> fieldsCaps,
179+
// The map won't contain entries without duplicates, i.e., it's number of occurrences - 1.
180+
Map<String, Integer> indexMappingHashDuplicates
181+
) {}
182+
183+
private static CollectedFieldCaps collectFieldCaps(FieldCapabilitiesResponse fieldCapsResponse) {
184+
Map<String, Integer> indexMappingHashToDuplicateCount = new HashMap<>();
185+
Map<String, IndexFieldCapabilitiesWithSourceHash> fieldsCaps = new HashMap<>();
186+
176187
for (FieldCapabilitiesIndexResponse response : fieldCapsResponse.getIndexResponses()) {
177-
if (seenHashes.add(response.getIndexMappingHash()) == false) {
188+
if (indexMappingHashToDuplicateCount.compute(response.getIndexMappingHash(), (k, v) -> v == null ? 1 : v + 1) > 1) {
178189
continue;
179190
}
180191
for (IndexFieldCapabilities fc : response.get().values()) {
181192
if (fc.isMetadatafield()) {
182193
// ESQL builds the metadata fields if they are asked for without using the resolution.
183194
continue;
184195
}
185-
List<IndexFieldCapabilities> all = fieldsCaps.computeIfAbsent(fc.name(), (_key) -> new ArrayList<>());
196+
List<IndexFieldCapabilities> all = fieldsCaps.computeIfAbsent(
197+
fc.name(),
198+
(_key) -> new IndexFieldCapabilitiesWithSourceHash(new ArrayList<>(), response.getIndexMappingHash())
199+
).fieldCapabilities;
186200
all.add(fc);
187201
}
188202
}
189-
return fieldsCaps;
203+
204+
var iterator = indexMappingHashToDuplicateCount.entrySet().iterator();
205+
while (iterator.hasNext()) {
206+
var next = iterator.next();
207+
if (next.getValue() <= 1) {
208+
iterator.remove();
209+
} else {
210+
next.setValue(next.getValue() - 1);
211+
}
212+
}
213+
214+
return new CollectedFieldCaps(fieldsCaps, indexMappingHashToDuplicateCount);
190215
}
191216

192217
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;
@@ -96,6 +97,7 @@
9697
import org.elasticsearch.xpack.esql.session.IndexResolver;
9798

9899
import java.io.IOException;
100+
import java.nio.charset.StandardCharsets;
99101
import java.time.Period;
100102
import java.util.ArrayList;
101103
import java.util.List;
@@ -3106,6 +3108,27 @@ public void testResolveInsist_multiIndexFieldPartiallyExistsWithMultiTypesNoKeyw
31063108
assertThat(attr.unresolvedMessage(), is(expected));
31073109
}
31083110

3111+
public void testResolveInsist_multiIndexSameMapping_fieldIsMapped() {
3112+
assumeTrue("Requires UNMAPPED FIELDS", EsqlCapabilities.Cap.UNMAPPED_FIELDS.isEnabled());
3113+
3114+
IndexResolution resolution = IndexResolver.mergedMappings(
3115+
"foo, bar",
3116+
new FieldCapabilitiesResponse(
3117+
List.of(
3118+
fieldCapabilitiesIndexResponse("foo", messageResponseMap("long")),
3119+
fieldCapabilitiesIndexResponse("bar", messageResponseMap("long"))
3120+
),
3121+
List.of()
3122+
)
3123+
);
3124+
var plan = analyze("FROM foo, bar | INSIST_🐔 message", analyzer(resolution, TEST_VERIFIER));
3125+
var limit = as(plan, Limit.class);
3126+
var insist = as(limit.child(), Insist.class);
3127+
var attribute = (FieldAttribute) EsqlTestUtils.singleValue(insist.output());
3128+
assertThat(attribute.name(), is("message"));
3129+
assertThat(attribute.dataType(), is(DataType.LONG));
3130+
}
3131+
31093132
public void testResolveInsist_multiIndexFieldPartiallyExistsWithMultiTypesWithKeyword_createsAnInvalidMappedField() {
31103133
assumeTrue("Requires UNMAPPED FIELDS", EsqlCapabilities.Cap.UNMAPPED_FIELDS.isEnabled());
31113134

@@ -3536,7 +3559,11 @@ private static FieldCapabilitiesIndexResponse fieldCapabilitiesIndexResponse(
35363559
String indexName,
35373560
Map<String, IndexFieldCapabilities> fields
35383561
) {
3539-
return new FieldCapabilitiesIndexResponse(indexName, indexName, fields, false, IndexMode.STANDARD);
3562+
String indexMappingHash = new String(
3563+
MessageDigests.sha256().digest(fields.toString().getBytes(StandardCharsets.UTF_8)),
3564+
StandardCharsets.UTF_8
3565+
);
3566+
return new FieldCapabilitiesIndexResponse(indexName, indexMappingHash, fields, false, IndexMode.STANDARD);
35403567
}
35413568

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

0 commit comments

Comments
 (0)