Fix TextFieldMapperTests (#104192)

server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java

@@ -684,10 +684,14 @@ public class KeywordFieldMapperTests extends MapperTestCase {
 
         @Override
         public SyntheticSourceExample example(int maxValues) {
+            return example(maxValues, false);
+        }
+
+        public SyntheticSourceExample example(int maxValues, boolean loadBlockFromSource) {
             if (randomBoolean()) {
                 Tuple<String, String> v = generateValue();
                 Object loadBlock = v.v2();
-                if (ignoreAbove != null && v.v2().length() > ignoreAbove) {
+                if (loadBlockFromSource == false && ignoreAbove != null && v.v2().length() > ignoreAbove) {
                     loadBlock = null;
                 }
                 return new SyntheticSourceExample(v.v1(), v.v2(), loadBlock, this::mapping);
@@ -704,9 +708,14 @@ public class KeywordFieldMapperTests extends MapperTestCase {
                 }
             });
             List<String> outList = store ? outPrimary : new HashSet<>(outPrimary).stream().sorted().collect(Collectors.toList());
-            List<String> loadBlock = docValues
-                ? new HashSet<>(outPrimary).stream().sorted().collect(Collectors.toList())
-                : List.copyOf(outList);
+            List<String> loadBlock;
+            if (loadBlockFromSource) {
+                loadBlock = in;
+            } else if (docValues) {
+                loadBlock = new HashSet<>(outPrimary).stream().sorted().collect(Collectors.toList());
+            } else {
+                loadBlock = List.copyOf(outList);
+            }
             Object loadBlockResult = loadBlock.size() == 1 ? loadBlock.get(0) : loadBlock;
             outList.addAll(outExtraValues);
             Object out = outList.size() == 1 ? outList.get(0) : outList;

server/src/test/java/org/elasticsearch/index/mapper/TextFieldMapperTests.java

@@ -97,30 +97,6 @@ public class TextFieldMapperTests extends MapperTestCase {
         return "value";
     }
 
-    @Override
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/104152")
-    public void testBlockLoaderFromColumnReader() throws IOException {
-        super.testBlockLoaderFromColumnReader();
-    }
-
-    @Override
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/104152")
-    public void testBlockLoaderFromRowStrideReader() throws IOException {
-        super.testBlockLoaderFromRowStrideReader();
-    }
-
-    @Override
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/104152")
-    public void testBlockLoaderFromColumnReaderWithSyntheticSource() throws IOException {
-        super.testBlockLoaderFromColumnReaderWithSyntheticSource();
-    }
-
-    @Override
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/104152")
-    public void testBlockLoaderFromRowStrideReaderWithSyntheticSource() throws IOException {
-        super.testBlockLoaderFromRowStrideReaderWithSyntheticSource();
-    }
-
     public final void testExistsQueryIndexDisabled() throws IOException {
         MapperService mapperService = createMapperService(fieldMapping(b -> {
             minimalMapping(b);
@@ -1145,8 +1121,9 @@ public class TextFieldMapperTests extends MapperTestCase {
         boolean storeTextField = randomBoolean();
         boolean storedKeywordField = storeTextField || randomBoolean();
         String nullValue = storeTextField || usually() ? null : randomAlphaOfLength(2);
+        Integer ignoreAbove = randomBoolean() ? null : between(10, 100);
         KeywordFieldMapperTests.KeywordSyntheticSourceSupport keywordSupport = new KeywordFieldMapperTests.KeywordSyntheticSourceSupport(
-            randomBoolean() ? null : between(10, 100),
+            ignoreAbove,
             storedKeywordField,
             nullValue,
             false == storeTextField
@@ -1154,25 +1131,33 @@ public class TextFieldMapperTests extends MapperTestCase {
         return new SyntheticSourceSupport() {
             @Override
             public SyntheticSourceExample example(int maxValues) {
-                SyntheticSourceExample delegate = keywordSupport.example(maxValues);
                 if (storeTextField) {
+                    SyntheticSourceExample delegate = keywordSupport.example(maxValues, true);
                     return new SyntheticSourceExample(
                         delegate.inputValue(),
-                        delegate.result(),
-                        delegate.result(),
+                        delegate.expectedForSyntheticSource(),
+                        delegate.expectedForBlockLoader(),
                         b -> b.field("type", "text").field("store", true)
                     );
                 }
-                return new SyntheticSourceExample(delegate.inputValue(), delegate.result(), delegate.blockLoaderResult(), b -> {
-                    b.field("type", "text");
-                    b.startObject("fields");
-                    {
-                        b.startObject(randomAlphaOfLength(4));
-                        delegate.mapping().accept(b);
+                // We'll load from _source if ignore_above is defined, otherwise we load from the keyword field.
+                boolean loadingFromSource = ignoreAbove != null;
+                SyntheticSourceExample delegate = keywordSupport.example(maxValues, loadingFromSource);
+                return new SyntheticSourceExample(
+                    delegate.inputValue(),
+                    delegate.expectedForSyntheticSource(),
+                    delegate.expectedForBlockLoader(),
+                    b -> {
+                        b.field("type", "text");
+                        b.startObject("fields");
+                        {
+                            b.startObject(randomAlphaOfLength(4));
+                            delegate.mapping().accept(b);
+                            b.endObject();
+                        }
                         b.endObject();
                     }
-                    b.endObject();
-                });
+                );
             }
 
             @Override
@@ -1371,7 +1356,9 @@ public class TextFieldMapperTests extends MapperTestCase {
         String parentName = mapper.mappingLookup().parentField(ft.name());
         if (parentName == null) {
             TextFieldMapper.TextFieldType text = (TextFieldType) ft;
-            return text.syntheticSourceDelegate() != null && text.syntheticSourceDelegate().hasDocValues();
+            return text.syntheticSourceDelegate() != null
+                && text.syntheticSourceDelegate().hasDocValues()
+                && text.canUseSyntheticSourceDelegateForQuerying();
         }
         MappedFieldType parent = mapper.fieldType(parentName);
         if (false == parent.typeName().equals(KeywordFieldMapper.CONTENT_TYPE)) {

test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java

@@ -1035,8 +1035,8 @@ public abstract class MapperTestCase extends MapperServiceTestCase {
 
     public record SyntheticSourceExample(
         CheckedConsumer<XContentBuilder, IOException> inputValue,
-        CheckedConsumer<XContentBuilder, IOException> result,
-        CheckedConsumer<XContentBuilder, IOException> blockLoaderResult,
+        CheckedConsumer<XContentBuilder, IOException> expectedForSyntheticSource,
+        CheckedConsumer<XContentBuilder, IOException> expectedForBlockLoader,
         CheckedConsumer<XContentBuilder, IOException> mapping
     ) {
         public SyntheticSourceExample(Object inputValue, Object result, CheckedConsumer<XContentBuilder, IOException> mapping) {
@@ -1063,22 +1063,15 @@ public abstract class MapperTestCase extends MapperServiceTestCase {
 
         private String expected() throws IOException {
             XContentBuilder b = JsonXContent.contentBuilder().startObject().field("field");
-            result.accept(b);
+            expectedForSyntheticSource.accept(b);
             return Strings.toString(b.endObject());
         }
 
-        private Object expectedParsed() throws IOException {
-            return XContentHelper.convertToMap(JsonXContent.jsonXContent, expected(), false).get("field");
-        }
-
-        private String expectedBlockLoader() throws IOException {
+        private Object expectedParsedForBlockLoader() throws IOException {
             XContentBuilder b = JsonXContent.contentBuilder().startObject().field("field");
-            blockLoaderResult.accept(b);
-            return Strings.toString(b.endObject());
-        }
-
-        private Object expectedParsedBlockLoader() throws IOException {
-            return XContentHelper.convertToMap(JsonXContent.jsonXContent, expectedBlockLoader(), false).get("field");
+            expectedForBlockLoader.accept(b);
+            String str = Strings.toString(b.endObject());
+            return XContentHelper.convertToMap(JsonXContent.jsonXContent, str, false).get("field");
         }
     }
 
@@ -1239,23 +1232,19 @@ public abstract class MapperTestCase extends MapperServiceTestCase {
         assertNoDocValueLoader(b -> b.startArray("field").endArray());
     }
 
-    // TextFieldMapperTests @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/104152")
-    public void testBlockLoaderFromColumnReader() throws IOException {
+    public final void testBlockLoaderFromColumnReader() throws IOException {
         testBlockLoader(false, true);
     }
 
-    // TextFieldMapperTests @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/104152")
-    public void testBlockLoaderFromRowStrideReader() throws IOException {
+    public final void testBlockLoaderFromRowStrideReader() throws IOException {
         testBlockLoader(false, false);
     }
 
-    // TextFieldMapperTests @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/104152")
-    public void testBlockLoaderFromColumnReaderWithSyntheticSource() throws IOException {
+    public final void testBlockLoaderFromColumnReaderWithSyntheticSource() throws IOException {
         testBlockLoader(true, true);
     }
 
     // Removed 'final' to silence this test in GeoPointFieldMapperTests, which does not support synthetic source completely
-    // TextFieldMapperTests @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/104152")
     public void testBlockLoaderFromRowStrideReaderWithSyntheticSource() throws IOException {
         testBlockLoader(true, false);
     }
@@ -1343,8 +1332,7 @@ public abstract class MapperTestCase extends MapperServiceTestCase {
                         inBlock = valuesConvert.apply(inBlock);
                     }
                 }
-                // If we're reading from _source we expect the order to be preserved, otherwise it's jumbled.
-                Object expected = loader instanceof BlockSourceReader ? example.expectedParsed() : example.expectedParsedBlockLoader();
+                Object expected = example.expectedParsedForBlockLoader();
                 if (List.of().equals(expected)) {
                     assertThat(inBlock, nullValue());
                     return;