Correct the condition to use a multi field block loader in text field block loader (#126718) (#127164)

Author: lkts

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

@@ -970,12 +970,11 @@ public boolean isAggregatable() {
         }
 
         /**
-         * Returns true if the delegate sub-field can be used for loading and querying (ie. either isIndexed or isStored is true)
+         * Returns true if the delegate sub-field can be used for loading.
+         * A delegate by definition must have doc_values or be stored so most of the time it can be used for loading.
          */
         public boolean canUseSyntheticSourceDelegateForLoading() {
-            return syntheticSourceDelegate != null
-                && syntheticSourceDelegate.ignoreAbove() == Integer.MAX_VALUE
-                && (syntheticSourceDelegate.isIndexed() || syntheticSourceDelegate.isStored());
+            return syntheticSourceDelegate != null && syntheticSourceDelegate.ignoreAbove() == Integer.MAX_VALUE;
         }
 
         /**

server/src/test/java/org/elasticsearch/index/mapper/blockloader/TextFieldBlockLoaderTests.java

@@ -35,20 +35,20 @@ protected Object expected(Map<String, Object> fieldMapping, Object value, TestCo
         var fields = (Map<String, Object>) fieldMapping.get("fields");
         if (fields != null) {
             var keywordMultiFieldMapping = (Map<String, Object>) fields.get("kwd");
+            Object normalizer = fields.get("normalizer");
             boolean docValues = hasDocValues(keywordMultiFieldMapping, true);
-            boolean index = keywordMultiFieldMapping.getOrDefault("index", true).equals(true);
             boolean store = keywordMultiFieldMapping.getOrDefault("store", false).equals(true);
             Object ignoreAbove = keywordMultiFieldMapping.get("ignore_above");
 
-            // See TextFieldMapper.SyntheticSourceHelper#usingSyntheticSourceDelegate
+            // See TextFieldMapper.SyntheticSourceHelper#getKeywordFieldMapperForSyntheticSource
             // and TextFieldMapper#canUseSyntheticSourceDelegateForLoading().
-            boolean usingSyntheticSourceDelegate = docValues || store;
-            boolean canUseSyntheticSourceDelegateForLoading = usingSyntheticSourceDelegate && ignoreAbove == null && (index || store);
+            boolean usingSyntheticSourceDelegate = normalizer == null && (docValues || store);
+            boolean canUseSyntheticSourceDelegateForLoading = usingSyntheticSourceDelegate && ignoreAbove == null;
             if (canUseSyntheticSourceDelegateForLoading) {
                 return KeywordFieldBlockLoaderTests.expectedValue(keywordMultiFieldMapping, value, params, testContext);
             }
 
-            // Even if multi-field is not eligible for loading it can still be used to produce synthetic source
+            // Even if multi field is not eligible for loading it can still be used to produce synthetic source
             // and then we load from the synthetic source.
             // Synthetic source is actually different from keyword field block loader results
             // because synthetic source includes values exceeding ignore_above and block loader doesn't.
@@ -66,9 +66,7 @@ protected Object expected(Map<String, Object> fieldMapping, Object value, TestCo
                 boolean textFieldIndexed = (boolean) fieldMapping.getOrDefault("index", true);
 
                 if (value == null) {
-                    if (textFieldIndexed == false
-                        && nullValue != null
-                        && (ignoreAbove == null || nullValue.length() <= (int) ignoreAbove)) {
+                    if (textFieldIndexed == false && nullValue != null && nullValue.length() <= (int) ignoreAbove) {
                         return new BytesRef(nullValue);
                     }
 
@@ -89,7 +87,7 @@ protected Object expected(Map<String, Object> fieldMapping, Object value, TestCo
                 var indexed = values.stream()
                     .map(s -> s == null ? nullValue : s)
                     .filter(Objects::nonNull)
-                    .filter(s -> ignoreAbove == null || s.length() <= (int) ignoreAbove)
+                    .filter(s -> s.length() <= (int) ignoreAbove)
                     .map(BytesRef::new)
                     .collect(Collectors.toList());
 
@@ -100,22 +98,20 @@ protected Object expected(Map<String, Object> fieldMapping, Object value, TestCo
                 }
 
                 // ignored values always come last
-                List<BytesRef> ignored = ignoreAbove == null
-                    ? List.of()
-                    : values.stream()
-                        .map(s -> s == null ? nullValue : s)
-                        .filter(Objects::nonNull)
-                        .filter(s -> s.length() > (int) ignoreAbove)
-                        .map(BytesRef::new)
-                        .toList();
+                List<BytesRef> ignored = values.stream()
+                    .map(s -> s == null ? nullValue : s)
+                    .filter(Objects::nonNull)
+                    .filter(s -> s.length() > (int) ignoreAbove)
+                    .map(BytesRef::new)
+                    .toList();
 
                 indexed.addAll(ignored);
 
                 return maybeFoldList(indexed);
             }
         }
 
-        // Loading from _ignored_source or stored _source
+        // Loading from stored field, _ignored_source or stored _source
         return valuesInSourceOrder(value);
     }