Build the correct keyword field type for PotentiallyUnmappedKeywordEsField (#129854)

martijnvg · web-flow · commit bd5f43f3eed4 · 2025-06-25T15:29:26.000+02:00
Before a `KeywordFieldType` was created that didn't set the isSyntheticSource field, causing to use the wrong block loader that would synthesize the complete _source instead of just loading values from ignored source. This PR addresses this.
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/EsPhysicalOperationProviders.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/EsPhysicalOperationProviders.java
@@ -7,13 +7,17 @@
 
 package org.elasticsearch.xpack.esql.planner;
 
+import org.apache.lucene.document.FieldType;
+import org.apache.lucene.index.DocValuesType;
+import org.apache.lucene.index.IndexOptions;
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.SortedSetDocValues;
 import org.apache.lucene.search.BooleanClause;
 import org.apache.lucene.search.BooleanQuery;
 import org.apache.lucene.search.IndexSearcher;
 import org.apache.lucene.search.Query;
 import org.elasticsearch.common.logging.HeaderWarning;
+import org.elasticsearch.common.lucene.Lucene;
 import org.elasticsearch.compute.aggregation.AggregatorMode;
 import org.elasticsearch.compute.aggregation.GroupingAggregator;
 import org.elasticsearch.compute.aggregation.blockhash.BlockHash;
@@ -76,7 +80,6 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
@@ -177,6 +180,13 @@ private BlockLoader getBlockLoaderFor(int shardId, Attribute attr, MappedFieldTy
 
     /** A hack to pretend an unmapped field still exists. */
     private static class DefaultShardContextForUnmappedField extends DefaultShardContext {
+        private static final FieldType UNMAPPED_FIELD_TYPE = new FieldType(KeywordFieldMapper.Defaults.FIELD_TYPE);
+        static {
+            UNMAPPED_FIELD_TYPE.setDocValuesType(DocValuesType.NONE);
+            UNMAPPED_FIELD_TYPE.setIndexOptions(IndexOptions.NONE);
+            UNMAPPED_FIELD_TYPE.setStored(false);
+            UNMAPPED_FIELD_TYPE.freeze();
+        }
         private final KeywordEsField unmappedEsField;
 
         DefaultShardContextForUnmappedField(DefaultShardContext ctx, PotentiallyUnmappedKeywordEsField unmappedEsField) {
@@ -187,9 +197,22 @@ private static class DefaultShardContextForUnmappedField extends DefaultShardCon
         @Override
         public @Nullable MappedFieldType fieldType(String name) {
             var superResult = super.fieldType(name);
-            return superResult == null && name.equals(unmappedEsField.getName())
-                ? new KeywordFieldMapper.KeywordFieldType(name, false /* isIndexed */, false /* hasDocValues */, Map.of() /* meta */)
-                : superResult;
+            return superResult == null && name.equals(unmappedEsField.getName()) ? createUnmappedFieldType(name, this) : superResult;
+        }
+
+        static MappedFieldType createUnmappedFieldType(String name, DefaultShardContext context) {
+            var builder = new KeywordFieldMapper.Builder(name, context.ctx.indexVersionCreated());
+            builder.docValues(false);
+            builder.indexed(false);
+            return new KeywordFieldMapper.KeywordFieldType(
+                name,
+                UNMAPPED_FIELD_TYPE,
+                Lucene.KEYWORD_ANALYZER,
+                Lucene.KEYWORD_ANALYZER,
+                Lucene.KEYWORD_ANALYZER,
+                builder,
+                context.ctx.isSourceSynthetic()
+            );
         }
     }
 
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlannerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlannerTests.java
@@ -23,13 +23,18 @@
 import org.elasticsearch.compute.lucene.DataPartitioning;
 import org.elasticsearch.compute.lucene.LuceneSourceOperator;
 import org.elasticsearch.compute.lucene.LuceneTopNSourceOperator;
+import org.elasticsearch.compute.lucene.ValuesSourceReaderOperator;
 import org.elasticsearch.compute.operator.SourceOperator;
 import org.elasticsearch.compute.test.TestBlockFactory;
 import org.elasticsearch.core.IOUtils;
 import org.elasticsearch.core.Releasable;
 import org.elasticsearch.core.Releasables;
 import org.elasticsearch.index.IndexMode;
 import org.elasticsearch.index.cache.query.TrivialQueryCachingPolicy;
+import org.elasticsearch.index.mapper.BlockLoader;
+import org.elasticsearch.index.mapper.BlockSourceReader;
+import org.elasticsearch.index.mapper.FallbackSyntheticSourceBlockLoader;
+import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.MapperServiceTestCase;
 import org.elasticsearch.node.Node;
 import org.elasticsearch.plugins.ExtensiblePlugin;
@@ -42,10 +47,12 @@
 import org.elasticsearch.xpack.esql.core.tree.Source;
 import org.elasticsearch.xpack.esql.core.type.DataType;
 import org.elasticsearch.xpack.esql.core.type.EsField;
+import org.elasticsearch.xpack.esql.core.type.PotentiallyUnmappedKeywordEsField;
 import org.elasticsearch.xpack.esql.core.util.StringUtils;
 import org.elasticsearch.xpack.esql.expression.Order;
 import org.elasticsearch.xpack.esql.index.EsIndex;
 import org.elasticsearch.xpack.esql.plan.physical.EsQueryExec;
+import org.elasticsearch.xpack.esql.plan.physical.FieldExtractExec;
 import org.elasticsearch.xpack.esql.plan.physical.LimitExec;
 import org.elasticsearch.xpack.esql.plan.physical.ParallelExec;
 import org.elasticsearch.xpack.esql.plugin.EsqlPlugin;
@@ -64,6 +71,7 @@
 
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.lessThanOrEqualTo;
 
 public class LocalExecutionPlannerTests extends MapperServiceTestCase {
@@ -84,10 +92,17 @@ public static Iterable<Object[]> parameters() throws Exception {
 
     private final ArrayList<Releasable> releasables = new ArrayList<>();
 
+    private Settings settings = SETTINGS;
+
     public LocalExecutionPlannerTests(@Name("estimatedRowSizeIsHuge") boolean estimatedRowSizeIsHuge) {
         this.estimatedRowSizeIsHuge = estimatedRowSizeIsHuge;
     }
 
+    @Override
+    protected Settings getIndexSettings() {
+        return settings;
+    }
+
     @Override
     protected Collection<Plugin> getPlugins() {
         var plugin = new SpatialPlugin();
@@ -229,6 +244,47 @@ public void testParallel() throws Exception {
         assertThat(plan.driverFactories, hasSize(2));
     }
 
+    public void testPlanUnmappedFieldExtractStoredSource() throws Exception {
+        var blockLoader = constructBlockLoader();
+        // In case of stored source we expect bytes based block source loader (this loads source from _source)
+        assertThat(blockLoader, instanceOf(BlockSourceReader.BytesRefsBlockLoader.class));
+    }
+
+    public void testPlanUnmappedFieldExtractSyntheticSource() throws Exception {
+        // Enables synthetic source, so that fallback synthetic source blocker loader is used:
+        settings = Settings.builder().put(settings).put("index.mapping.source.mode", "synthetic").build();
+
+        var blockLoader = constructBlockLoader();
+        // In case of synthetic source we expect bytes based block source loader (this loads source from _ignored_source)
+        assertThat(blockLoader, instanceOf(FallbackSyntheticSourceBlockLoader.class));
+    }
+
+    private BlockLoader constructBlockLoader() throws IOException {
+        EsQueryExec queryExec = new EsQueryExec(
+            Source.EMPTY,
+            index().name(),
+            IndexMode.STANDARD,
+            index().indexNameWithModes(),
+            List.of(new FieldAttribute(Source.EMPTY, EsQueryExec.DOC_ID_FIELD.getName(), EsQueryExec.DOC_ID_FIELD)),
+            null,
+            null,
+            null,
+            between(1, 1000)
+        );
+        FieldExtractExec fieldExtractExec = new FieldExtractExec(
+            Source.EMPTY,
+            queryExec,
+            List.of(
+                new FieldAttribute(Source.EMPTY, "potentially_unmapped", new PotentiallyUnmappedKeywordEsField("potentially_unmapped"))
+            ),
+            MappedFieldType.FieldExtractPreference.NONE
+        );
+        LocalExecutionPlanner.LocalExecutionPlan plan = planner().plan("test", FoldContext.small(), fieldExtractExec);
+        var p = plan.driverFactories.get(0).driverSupplier().physicalOperation();
+        var fieldInfo = ((ValuesSourceReaderOperator.Factory) p.intermediateOperatorFactories.get(0)).fields().get(0);
+        return fieldInfo.blockLoader().apply(0);
+    }
+
     private int randomEstimatedRowSize(boolean huge) {
         int hugeBoundary = SourceOperator.MIN_TARGET_PAGE_SIZE * 10;
         return huge ? between(hugeBoundary, Integer.MAX_VALUE) : between(1, hugeBoundary);
diff --git a/x-pack/plugin/logsdb/src/yamlRestTest/resources/rest-api-spec/test/52_esql_insist_operator_synthetic_source.yml b/x-pack/plugin/logsdb/src/yamlRestTest/resources/rest-api-spec/test/52_esql_insist_operator_synthetic_source.yml
@@ -0,0 +1,94 @@
+---
+setup:
+  - do:
+      indices.create:
+        index: my-index
+        body:
+          settings:
+            index:
+              mode: logsdb
+          mappings:
+            dynamic: false
+            properties:
+              "@timestamp":
+                type: date
+              message:
+                type: text
+
+  - do:
+      bulk:
+        index: my-index
+        refresh: true
+        body:
+          - { "index": { } }
+          - { "@timestamp": "2024-02-12T10:30:00Z", "host.name": "foo", "agent_id": "darth-vader", "process_id": 101, "http_method": "GET", "is_https": false, "location": {"lat" : 40.7128, "lon" : -74.0060}, "message": "No, I am your father." }
+          - { "index": { } }
+          - { "@timestamp": "2024-02-12T10:31:00Z", "host.name": "bar", "agent_id": "yoda", "process_id": 102, "http_method": "PUT", "is_https": false, "location": {"lat" : 40.7128, "lon" : -74.0060}, "message": "Do. Or do not. There is no try." }
+          - { "index": { } }
+          - { "@timestamp": "2024-02-12T10:32:00Z", "host.name": "foo", "agent_id": "obi-wan", "process_id": 103, "http_method": "GET", "is_https": false, "location": {"lat" : 40.7128, "lon" : -74.0060}, "message": "May the force be with you." }
+          - { "index": { } }
+          - { "@timestamp": "2024-02-12T10:33:00Z", "host.name": "baz", "agent_id": "darth-vader", "process_id": 102, "http_method": "POST", "is_https": true, "location": {"lat" : 40.7128, "lon" : -74.0060}, "message": "I find your lack of faith disturbing." }
+          - { "index": { } }
+          - { "@timestamp": "2024-02-12T10:34:00Z", "host.name": "baz", "agent_id": "yoda", "process_id": 104, "http_method": "POST", "is_https": false, "location": {"lat" : 40.7128, "lon" : -74.0060}, "message": "Wars not make one great." }
+          - { "index": { } }
+          - { "@timestamp": "2024-02-12T10:35:00Z", "host.name": "foo", "agent_id": "obi-wan", "process_id": 105, "http_method": "GET", "is_https": false, "location": {"lat" : 40.7128, "lon" : -74.0060}, "message": "That's no moon. It's a space station." }
+
+---
+teardown:
+  - do:
+      indices.delete:
+        index: my-index
+
+---
+"Simple from":
+  - do:
+      esql.query:
+        body:
+          query: 'FROM my-index | SORT @timestamp | LIMIT 1'
+
+  - match: {columns.0.name: "@timestamp"}
+  - match: {columns.0.type: "date"}
+  - match: {columns.1.name: "message"}
+  - match: {columns.1.type: "text"}
+
+  - match: {values.0.0: "2024-02-12T10:30:00.000Z"}
+  - match: {values.0.1: "No, I am your father."}
+
+---
+"FROM with INSIST_🐔and LIMIT 1":
+  - do:
+      esql.query:
+        body:
+          query: 'FROM my-index | INSIST_🐔 host.name, agent_id, http_method | SORT @timestamp | KEEP host.name, agent_id, http_method | LIMIT 1'
+
+  - match: {columns.0.name: "host.name"}
+  - match: {columns.0.type: "keyword"}
+  - match: {columns.1.name: "agent_id"}
+  - match: {columns.1.type: "keyword"}
+  - match: {columns.2.name: "http_method"}
+  - match: {columns.2.type: "keyword"}
+
+  - match: {values.0.0: "foo"}
+  - match: {values.0.1: "darth-vader"}
+  - match: {values.0.2: "GET"}
+
+---
+"FROM with INSIST_🐔":
+  - requires:
+      test_runner_features: allowed_warnings_regex
+  - do:
+      allowed_warnings_regex:
+        - "No limit defined, adding default limit of \\[.*\\]"
+      esql.query:
+        body:
+          query: 'FROM my-index | INSIST_🐔 agent_id | SORT @timestamp | KEEP agent_id'
+
+  - match: {columns.0.name: "agent_id"}
+  - match: {columns.0.type: "keyword"}
+
+  - match: {values.0.0: "darth-vader"}
+  - match: {values.1.0: "yoda"}
+  - match: {values.2.0: "obi-wan"}
+  - match: {values.3.0: "darth-vader"}
+  - match: {values.4.0: "yoda"}
+  - match: {values.5.0: "obi-wan"}
