Skip to content

Commit 189df72

Browse files
Mikep86Mikep86
committed
Fix inference fields handling on old indices (elastic#136312)
1 parent 356bec2 commit 189df72

File tree

5 files changed

+477
-12
lines changed

5 files changed

+477
-12
lines changed

‎docs/changelog/136312.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
pr: 136312
2+
summary: Fix _inference_fields handling on old indices
3+
area: Vector Search
4+
type: bug
5+
issues: [
6+
136130
7+
]

‎server/src/main/java/org/elasticsearch/index/get/ShardGetService.java‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -311,8 +311,7 @@ private GetResult innerGetFetch(
311311
fetchSourceContext = res.v1();
312312
}
313313

314-
if (mappingLookup.inferenceFields().isEmpty() == false
315-
&& shouldExcludeInferenceFieldsFromSource(indexSettings, fetchSourceContext) == false) {
314+
if (mappingLookup.inferenceFields().isEmpty() == false && shouldExcludeInferenceFieldsFromSource(fetchSourceContext) == false) {
316315
storedFieldSet.add(InferenceMetadataFieldsMapper.NAME);
317316
}
318317

@@ -424,17 +423,30 @@ private static Boolean shouldExcludeVectorsFromSourceExplicit(FetchSourceContext
424423
return fetchSourceContext != null ? fetchSourceContext.excludeVectors() : null;
425424
}
426425

427-
public static boolean shouldExcludeInferenceFieldsFromSource(IndexSettings indexSettings, FetchSourceContext fetchSourceContext) {
428-
var explicit = shouldExcludeInferenceFieldsFromSourceExplicit(fetchSourceContext);
429-
var filter = fetchSourceContext != null ? fetchSourceContext.filter() : null;
430-
if (filter != null) {
431-
if (filter.isPathFiltered(InferenceMetadataFieldsMapper.NAME, true)) {
426+
public static boolean shouldExcludeInferenceFieldsFromSource(FetchSourceContext fetchSourceContext) {
427+
if (fetchSourceContext != null) {
428+
if (fetchSourceContext.fetchSource() == false) {
429+
// Source is disabled
432430
return true;
433-
} else if (filter.isExplicitlyIncluded(InferenceMetadataFieldsMapper.NAME)) {
434-
return false;
431+
}
432+
433+
var filter = fetchSourceContext.filter();
434+
if (filter != null) {
435+
if (filter.isPathFiltered(InferenceMetadataFieldsMapper.NAME, true)) {
436+
return true;
437+
} else if (filter.isExplicitlyIncluded(InferenceMetadataFieldsMapper.NAME)) {
438+
return false;
439+
}
440+
}
441+
442+
Boolean excludeInferenceFieldsExplicit = shouldExcludeInferenceFieldsFromSourceExplicit(fetchSourceContext);
443+
if (excludeInferenceFieldsExplicit != null) {
444+
return excludeInferenceFieldsExplicit;
435445
}
436446
}
437-
return explicit != null ? explicit : INDEX_MAPPING_EXCLUDE_SOURCE_VECTORS_SETTING.get(indexSettings.getSettings());
447+
448+
// We always default to excluding the inference metadata field, unless the fetch source context says otherwise
449+
return true;
438450
}
439451

440452
private static Boolean shouldExcludeInferenceFieldsFromSourceExplicit(FetchSourceContext fetchSourceContext) {

‎server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,7 @@ private SearchHits buildSearchHits(SearchContext context, int[] docIdsToLoad, Pr
134134
context.fetchSourceContext(res.v1());
135135
}
136136

137-
if (lookup.inferenceFields().isEmpty() == false
138-
&& shouldExcludeInferenceFieldsFromSource(context.indexShard().indexSettings(), context.fetchSourceContext()) == false) {
137+
if (lookup.inferenceFields().isEmpty() == false && shouldExcludeInferenceFieldsFromSource(context.fetchSourceContext()) == false) {
139138
// Rehydrate the inference fields into the {@code _source} because they were explicitly requested.
140139
var oldFetchFieldsContext = context.fetchFieldsContext();
141140
var newFetchFieldsContext = new FetchFieldsContext(new ArrayList<>());

‎server/src/test/java/org/elasticsearch/index/shard/ShardGetServiceTests.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,12 @@
2424
import org.elasticsearch.index.engine.TranslogOperationAsserter;
2525
import org.elasticsearch.index.engine.VersionConflictEngineException;
2626
import org.elasticsearch.index.get.GetResult;
27+
import org.elasticsearch.index.get.ShardGetService;
28+
import org.elasticsearch.index.mapper.InferenceMetadataFieldsMapper;
2729
import org.elasticsearch.index.mapper.RoutingFieldMapper;
2830
import org.elasticsearch.index.translog.Translog;
2931
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
32+
import org.elasticsearch.search.lookup.SourceFilter;
3033
import org.elasticsearch.xcontent.XContentBuilder;
3134
import org.elasticsearch.xcontent.XContentFactory;
3235
import org.elasticsearch.xcontent.XContentParser;
@@ -411,6 +414,16 @@ public void testGetFromTranslog() throws IOException {
411414
closeShards(primary);
412415
}
413416

417+
public void testShouldExcludeInferenceFieldsFromSource() {
418+
for (int i = 0; i < 100; i++) {
419+
ExcludeInferenceFieldsTestScenario scenario = new ExcludeInferenceFieldsTestScenario();
420+
assertThat(
421+
ShardGetService.shouldExcludeInferenceFieldsFromSource(scenario.fetchSourceContext),
422+
equalTo(scenario.shouldExcludeInferenceFields())
423+
);
424+
}
425+
}
426+
414427
Translog.Index toIndexOp(String source) throws IOException {
415428
XContentParser parser = createParser(XContentType.JSON.xContent(), source);
416429
XContentBuilder builder = XContentFactory.jsonBuilder();
@@ -425,4 +438,74 @@ Translog.Index toIndexOp(String source) throws IOException {
425438
IndexRequest.UNSET_AUTO_GENERATED_TIMESTAMP
426439
);
427440
}
441+
442+
private static class ExcludeInferenceFieldsTestScenario {
443+
private final FetchSourceContext fetchSourceContext;
444+
445+
private ExcludeInferenceFieldsTestScenario() {
446+
this.fetchSourceContext = generateRandomFetchSourceContext();
447+
}
448+
449+
private boolean shouldExcludeInferenceFields() {
450+
if (fetchSourceContext != null) {
451+
if (fetchSourceContext.fetchSource() == false) {
452+
return true;
453+
}
454+
455+
SourceFilter filter = fetchSourceContext.filter();
456+
if (filter != null) {
457+
if (Arrays.asList(filter.getExcludes()).contains(InferenceMetadataFieldsMapper.NAME)) {
458+
return true;
459+
} else if (filter.getIncludes().length > 0) {
460+
return Arrays.asList(filter.getIncludes()).contains(InferenceMetadataFieldsMapper.NAME) == false;
461+
}
462+
}
463+
464+
Boolean excludeInferenceFieldsExplicit = fetchSourceContext.excludeInferenceFields();
465+
if (excludeInferenceFieldsExplicit != null) {
466+
return excludeInferenceFieldsExplicit;
467+
}
468+
}
469+
470+
return true;
471+
}
472+
473+
private static FetchSourceContext generateRandomFetchSourceContext() {
474+
FetchSourceContext fetchSourceContext = switch (randomIntBetween(0, 4)) {
475+
case 0 -> FetchSourceContext.FETCH_SOURCE;
476+
case 1 -> FetchSourceContext.FETCH_ALL_SOURCE;
477+
case 2 -> FetchSourceContext.FETCH_ALL_SOURCE_EXCLUDE_INFERENCE_FIELDS;
478+
case 3 -> FetchSourceContext.DO_NOT_FETCH_SOURCE;
479+
case 4 -> null;
480+
default -> throw new IllegalStateException("Unhandled randomized case");
481+
};
482+
483+
if (fetchSourceContext != null && fetchSourceContext.fetchSource()) {
484+
String[] includes = null;
485+
String[] excludes = null;
486+
if (randomBoolean()) {
487+
// Randomly include a non-existent field to test explicit inclusion handling
488+
String field = randomBoolean() ? InferenceMetadataFieldsMapper.NAME : randomIdentifier();
489+
includes = new String[] { field };
490+
}
491+
if (randomBoolean()) {
492+
// Randomly exclude a non-existent field to test implicit inclusion handling
493+
String field = randomBoolean() ? InferenceMetadataFieldsMapper.NAME : randomIdentifier();
494+
excludes = new String[] { field };
495+
}
496+
497+
if (includes != null || excludes != null) {
498+
fetchSourceContext = FetchSourceContext.of(
499+
fetchSourceContext.fetchSource(),
500+
fetchSourceContext.excludeVectors(),
501+
fetchSourceContext.excludeInferenceFields(),
502+
includes,
503+
excludes
504+
);
505+
}
506+
}
507+
508+
return fetchSourceContext;
509+
}
510+
}
428511
}

0 commit comments

Comments
 (0)