removed fallback synthetic source block loader being used

Author: Kubik42

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java

@@ -680,16 +680,18 @@ protected void parseCreateField(DocumentParserContext context) throws IOExceptio
 
         // match_only_text isn't stored, so if synthetic source needs to be supported, we must do something about it
         if (needsToSupportSyntheticSource()) {
-            // if the delegate can't support synthetic source for the given value, then store a copy of said value so
-            // that synthetic source can load it
-            if (fieldType().textFieldType.canUseSyntheticSourceDelegateForSyntheticSource(value.string()) == false) {
-                final String fieldName = fieldType().syntheticSourceFallbackFieldName();
-                if (storedFieldInBinaryFormat) {
-                    final var bytesRef = new BytesRef(utfBytes.bytes(), utfBytes.offset(), utfBytes.length());
-                    context.doc().add(new StoredField(fieldName, bytesRef));
-                } else {
-                    context.doc().add(new StoredField(fieldName, value.string()));
-                }
+            // check if we can use the delegate
+            if (fieldType().textFieldType.canUseSyntheticSourceDelegateForSyntheticSource(value.string())) {
+                return;
+            }
+
+            // if not, then store this field explicitly so that synthetic source can load it
+            final String fieldName = fieldType().syntheticSourceFallbackFieldName();
+            if (storedFieldInBinaryFormat) {
+                final var bytesRef = new BytesRef(utfBytes.bytes(), utfBytes.offset(), utfBytes.length());
+                context.doc().add(new StoredField(fieldName, bytesRef));
+            } else {
+                context.doc().add(new StoredField(fieldName, value.string()));
             }
         }
     }

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/mapping/20_synthetic_source.yml

@@ -160,6 +160,28 @@ synthetic_source text with ignored multi-field and multiple values in the same d
             match_phrase:
               foo: this value is
 
-  - match: { "hits.total.value": 1 }
-  - match: { hits.hits.0._source.foo.0: "This value is short" }
-  - match: { hits.hits.0._source.foo.1: "This value is too long and will be ignored" }
+  - match: { hits.total.value: 1 }
+  - match: { hits.hits.0._source.foo.0: "This value is too long and will be ignored" }
+  - match: { hits.hits.0._source.foo.1: "This value is short" }
+
+  # flip around the two values, now short first followed by a long one
+  - do:
+      index:
+        index: synthetic_source_test
+        id:    "1"
+        refresh: true
+        body:
+          foo: [ "a short potato", "a very looooooooooooooong potato" ]
+
+  - do:
+      search:
+        index: synthetic_source_test
+        body:
+          query:
+            match_phrase:
+              foo: potato
+
+  - match: { hits.total.value: 1 }
+  # the order is flipped because the keyword field loader is added after the parent field loader
+  - match: { hits.hits.0._source.foo.0: "a very looooooooooooooong potato" }
+  - match: { hits.hits.0._source.foo.1: "a short potato" }

server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java

@@ -75,7 +75,6 @@
 import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
 import org.elasticsearch.xcontent.ToXContent;
 import org.elasticsearch.xcontent.XContentBuilder;
-import org.elasticsearch.xcontent.XContentParser;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -1008,9 +1007,8 @@ public boolean canUseSyntheticSourceDelegateForQuerying() {
          */
         public boolean canUseSyntheticSourceDelegateForSyntheticSource(final String value) {
             if (syntheticSourceDelegate.isPresent()) {
-                KeywordFieldMapper.KeywordFieldType kwdFieldType = syntheticSourceDelegate.get();
                 // if the keyword field is going to be ignored, then we can't rely on it for synthetic source
-                return kwdFieldType.isIgnored(value) == false;
+                return syntheticSourceDelegate.get().isIgnored(value) == false;
             }
             return false;
         }
@@ -1064,60 +1062,10 @@ protected String delegatingTo() {
                 return new BlockStoredFieldsReader.BytesFromStringsBlockLoader(name());
             }
 
-            // _ignored_source field will contain entries for this field if it is not stored
-            // and there is no syntheticSourceDelegate.
-            // See #syntheticSourceSupport().
-            // But if a text field is a multi field it won't have an entry in _ignored_source.
-            // The parent might, but we don't have enough context here to figure this out.
-            // So we bail.
-            if (isSyntheticSourceEnabled && syntheticSourceDelegate.isEmpty() && parentField == null) {
-                return fallbackSyntheticSourceBlockLoader(blContext);
-            }
-
             SourceValueFetcher fetcher = SourceValueFetcher.toString(blContext.sourcePaths(name()));
             return new BlockSourceReader.BytesRefsBlockLoader(fetcher, blockReaderDisiLookup(blContext));
         }
 
-        FallbackSyntheticSourceBlockLoader fallbackSyntheticSourceBlockLoader(BlockLoaderContext blContext) {
-            var reader = new FallbackSyntheticSourceBlockLoader.SingleValueReader<BytesRef>(null) {
-                @Override
-                public void convertValue(Object value, List<BytesRef> accumulator) {
-                    if (value != null) {
-                        accumulator.add(new BytesRef(value.toString()));
-                    }
-                }
-
-                @Override
-                protected void parseNonNullValue(XContentParser parser, List<BytesRef> accumulator) throws IOException {
-                    var text = parser.textOrNull();
-
-                    if (text != null) {
-                        accumulator.add(new BytesRef(text));
-                    }
-                }
-
-                @Override
-                public void writeToBlock(List<BytesRef> values, BlockLoader.Builder blockBuilder) {
-                    var bytesRefBuilder = (BlockLoader.BytesRefBuilder) blockBuilder;
-
-                    for (var value : values) {
-                        bytesRefBuilder.appendBytesRef(value);
-                    }
-                }
-            };
-
-            return new FallbackSyntheticSourceBlockLoader(
-                reader,
-                name(),
-                IgnoredSourceFieldMapper.ignoredSourceFormat(blContext.indexSettings().getIndexVersionCreated())
-            ) {
-                @Override
-                public Builder builder(BlockFactory factory, int expectedCount) {
-                    return factory.bytesRefs(expectedCount);
-                }
-            };
-        }
-
         /**
          * Build an iterator of documents that have the field. This mirrors parseCreateField,
          * using whatever