[9.3] Fix incorrect nullify with unmapped fields (#142300)

docs/changelog/142300.yaml

@@ -0,0 +1,6 @@
+area: ES|QL
+issues:
+ - 141870
+pr: 142300
+summary: Fix incorrect nullify with unmapped fields
+type: bug

x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java

@@ -370,7 +370,7 @@ protected boolean supportsInferenceTestServiceOnLocalCluster() {
     }
 
     @Override
-    protected boolean supportsIndexModeLookup() throws IOException {
+    protected boolean supportsIndexModeLookup() {
         return hasCapabilities(adminClient(), List.of(JOIN_LOOKUP_V12.capabilityName()));
     }
 
@@ -380,7 +380,7 @@ protected boolean supportsSourceFieldMapping() {
     }
 
     @Override
-    protected boolean supportsTook() throws IOException {
+    protected boolean supportsTook() {
         // We don't read took properly in multi-cluster tests.
         return false;
     }

x-pack/plugin/esql/qa/server/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/multi_node/EsqlSpecIT.java

@@ -12,7 +12,6 @@
 import org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase;
 import org.junit.ClassRule;
 
-import java.io.IOException;
 import java.util.Map;
 
 public class EsqlSpecIT extends EsqlSpecTestCase {
@@ -39,7 +38,7 @@ protected boolean enableRoundingDoubleValuesOnAsserting() {
     }
 
     @Override
-    protected boolean supportsSourceFieldMapping() throws IOException {
+    protected boolean supportsSourceFieldMapping() {
         return false;
     }
 }

x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/EsqlSpecIT.java

@@ -68,6 +68,17 @@ protected boolean supportsTDigestField() {
         return RestEsqlTestCase.hasCapabilities(client(), List.of(EsqlCapabilities.Cap.TDIGEST_TECH_PREVIEW.capabilityName()));
     }
 
+    @Override
+    protected String maybeRandomizeQuery(String query) {
+        // TODO we should implement more generic randomization for SET parameters
+        return randomBoolean()
+            && testCase.expectedWarnings().isEmpty() // avoid shifting warnings positions in source query
+            && testCase.expectedWarningsRegex().isEmpty() // regexp might also contain line/position
+            && query.startsWith("SET") == false // avoid conflicts with provided settings
+                ? "SET unmapped_fields=\"nullify\"; " + query
+                : query;
+    }
+
     @Before
     public void configureChunks() throws IOException {
         assumeTrue("test clusters were broken", testClustersOk);

x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java

@@ -309,14 +309,18 @@ protected boolean requiresInferenceEndpointOnLocalCluster() {
         ).anyMatch(testCase.requiredCapabilities::contains);
     }
 
-    protected boolean supportsIndexModeLookup() throws IOException {
+    protected boolean supportsIndexModeLookup() {
         return true;
     }
 
-    protected boolean supportsSourceFieldMapping() throws IOException {
+    protected boolean supportsSourceFieldMapping() {
         return true;
     }
 
+    protected String maybeRandomizeQuery(String query) {
+        return query;
+    }
+
     protected boolean supportsExponentialHistograms() {
         return RestEsqlTestCase.hasCapabilities(
             client(),
@@ -341,6 +345,8 @@ protected void doTest() throws Throwable {
     }
 
     protected final void doTest(String query) throws Throwable {
+        query = maybeRandomizeQuery(query);
+
         RequestObjectBuilder builder = new RequestObjectBuilder(randomFrom(XContentType.values()));
 
         if (query.toUpperCase(Locale.ROOT).contains("LOOKUP_\uD83D\uDC14")) {
@@ -598,7 +604,7 @@ private Map<String, Map<String, RestEsqlTestCase.TypeAndValues>> tables() {
         return tables;
     }
 
-    protected boolean supportsTook() throws IOException {
+    protected boolean supportsTook() {
         if (supportsTook == null) {
             supportsTook = hasCapabilities(adminClient(), List.of("usage_contains_took"));
         }

x-pack/plugin/esql/qa/testFixtures/src/main/resources/date_nanos.csv-spec

@@ -38,7 +38,7 @@ nanos:date_nanos
 mv_min on date nanos
 required_capability: date_nanos_type
 
-FROM date_nanos | SORT millis ASC | WHERE millis < "2000-01-01" | EVAL nanos = MV_MIN(nanos) |  KEEP nanos; 
+FROM date_nanos | SORT millis ASC | WHERE millis < "2000-01-01" | EVAL nanos = MV_MIN(nanos) | KEEP nanos; 
 
 nanos:date_nanos
 2023-03-23T12:15:03.360103847Z
@@ -57,7 +57,7 @@ ct:integer
 mv_first on date nanos
 required_capability: date_nanos_type
 
-FROM date_nanos | SORT millis ASC | WHERE millis < "2000-01-01" | EVAL nanos = MV_FIRST(nanos) |  KEEP nanos;
+FROM date_nanos | SORT millis ASC | WHERE millis < "2000-01-01" | EVAL nanos = MV_FIRST(nanos) | KEEP nanos;
 
 nanos:date_nanos
 2023-03-23T12:15:03.360103847Z

x-pack/plugin/esql/qa/testFixtures/src/main/resources/unmapped-nullify.csv-spec

@@ -1,4 +1,4 @@
-simpleKeep
+keepUnmapped
 required_capability: optional_fields_nullify_tech_preview
 
 SET unmapped_fields="nullify"\;
@@ -13,6 +13,18 @@ null
 null
 ;
 
+keepMapped
+required_capability: date_nanos_type
+required_capability: optional_fields_nullify_tech_preview
+required_capability: optional_fields_fix_unmapped_field_detection
+
+SET unmapped_fields="nullify"; FROM date_nanos | SORT millis ASC | WHERE millis < "2000-01-01" | EVAL nanos = MV_MIN(nanos) | KEEP nanos; 
+
+nanos:date_nanos
+2023-03-23T12:15:03.360103847Z
+2023-01-23T13:55:01.543123456Z
+;
+
 keepStar
 required_capability: optional_fields_nullify_tech_preview
 

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

@@ -171,6 +171,12 @@ public enum Cap {
          */
         OPTIONAL_FIELDS_NULLIFY_TECH_PREVIEW,
 
+        /**
+         * Fix incorrect detection of unmapped fields in nullify/load mode when unresolved attributes
+         * match fields already present in the children's output.
+         */
+        OPTIONAL_FIELDS_FIX_UNMAPPED_FIELD_DETECTION,
+
         /**
          * Support specifically for *just* the _index METADATA field. Used by CsvTests, since that is the only metadata field currently
          * supported.

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/rules/ResolveUnmapped.java

@@ -78,15 +78,27 @@ private static LogicalPlan resolve(LogicalPlan plan, boolean load) {
         if (plan.childrenResolved() == false) {
             return plan;
         }
-
         var unresolved = collectUnresolved(plan);
         if (unresolved.isEmpty()) {
             return plan;
         }
-        var unresolvedLinkedSet = unresolvedLinkedSet(unresolved);
 
-        var transformed = load ? load(plan, unresolvedLinkedSet) : nullify(plan, unresolvedLinkedSet);
+        // Filter out unresolved attributes that exist in the children's output. These attributes are not truly unmapped;
+        // they just haven't been resolved yet by ResolveRefs (e.g. because the children only became resolved after ImplicitCasting).
+        // ResolveRefs will wire them up in the next iteration of the resolution batch.
+        Set<String> childOutputNames = new java.util.HashSet<>();
+        for (LogicalPlan child : plan.children()) {
+            for (Attribute attr : child.output()) {
+                childOutputNames.add(attr.name());
+            }
+        }
+        unresolved.removeIf(ua -> childOutputNames.contains(ua.name()));
+        if (unresolved.isEmpty()) {
+            return plan;
+        }
 
+        var unresolvedLinkedSet = unresolvedLinkedSet(unresolved);
+        var transformed = load ? load(plan, unresolvedLinkedSet) : nullify(plan, unresolvedLinkedSet);
         return transformed.equals(plan) ? plan : refreshPlan(transformed, unresolved);
     }