ESQL: Fix projection generation when pruning left join (#135446)

bpintea · web-flow · commit 2af97f4b9816 · 2025-10-07T14:23:22.000+03:00
Currently, in case the join key is (locally) null, the whole join is replaced with an `Eval` and a `Project`. The project however contain only reference attributes. This is insufficient, in case the attribute should point to another attribute pointing to a literal `null` added in the `Eval`.

This fixes that by allowing the generated `Project` to contain `NamedExpression`s, which can then be proper `Alias`es.
This is done by adjusting the join output calculation to work with the `NamedExpression`s, instead of just `Attribute`s.
diff --git a/docs/changelog/135446.yaml b/docs/changelog/135446.yaml
@@ -0,0 +1,5 @@
+pr: 135446
+summary: Fix projection generation when pruning left join
+area: ES|QL
+type: bug
+issues: []
diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/AttributeMap.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/AttributeMap.java
@@ -393,6 +393,12 @@ public static <E> AttributeMap<E> of(Attribute key, E value) {
         return map;
     }
 
+    public static <E> AttributeMap<E> mapAll(Collection<? extends E> collection, Function<E, Attribute> keyMapper) {
+        final AttributeMap<E> map = new AttributeMap<>();
+        collection.forEach(e -> map.add(keyMapper.apply(e), e));
+        return map;
+    }
+
     public static <E> Builder<E> builder() {
         return new Builder<>(new AttributeMap<>());
     }
diff --git a/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/AttributeMapTests.java b/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/AttributeMapTests.java
@@ -344,4 +344,18 @@ public void testValuesIteratorRemoval() {
         it.remove();
         assertThat(it.hasNext(), is(false));
     }
+
+    public void testMappAll() {
+        var one = a("one");
+        var two = a("two");
+        var three = a("three");
+
+        Collection<Attribute> collection = asList(one, two, three);
+        var map = AttributeMap.mapAll(collection, NamedExpression::toAttribute);
+
+        var builder = AttributeMap.builder();
+        collection.forEach(e -> builder.put(e, e.toAttribute()));
+
+        assertThat(map, is(builder.build()));
+    }
 }
diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java
@@ -89,15 +89,16 @@ public abstract class RestEsqlTestCase extends ESRestTestCase {
 
     private static final Logger LOGGER = LogManager.getLogger(RestEsqlTestCase.class);
 
+    private static final String MAPPING_FIELD;
     private static final String MAPPING_ALL_TYPES;
-
     private static final String MAPPING_ALL_TYPES_LOOKUP;
 
     static {
         String properties = EsqlTestUtils.loadUtf8TextFile("/mapping-all-types.json");
-        MAPPING_ALL_TYPES = "{\"mappings\": " + properties + "}";
-        String settings = "{\"settings\" : {\"mode\" : \"lookup\"}";
-        MAPPING_ALL_TYPES_LOOKUP = settings + ", " + "\"mappings\": " + properties + "}";
+        MAPPING_FIELD = "\"mappings\": " + properties;
+        MAPPING_ALL_TYPES = "{" + MAPPING_FIELD + "}";
+        String settings = "\"settings\" : {\"mode\" : \"lookup\"}";
+        MAPPING_ALL_TYPES_LOOKUP = "{" + settings + ", " + MAPPING_FIELD + "}";
     }
 
     private static final String DOCUMENT_TEMPLATE = """
@@ -1154,6 +1155,24 @@ public void testMultipleBatchesWithLookupJoin() throws IOException {
         }
     }
 
+    public void testPruneLeftJoinOnNullMatchingFieldAndShadowingAttributes() throws IOException {
+        var standardIndexName = "standard";
+        createIndex(standardIndexName, false, MAPPING_FIELD);
+        createIndex(testIndexName(), true);
+
+        var query = format(
+            null,
+            "FROM {}* | EVAL keyword = null::KEYWORD | LOOKUP JOIN {} ON keyword | KEEP keyword, integer, alias_integer | SORT keyword",
+            standardIndexName,
+            testIndexName()
+        );
+        Map<String, Object> result = runEsql(requestObjectBuilder().query(query));
+        var values = as(result.get("values"), List.class);
+        assertThat(values.size(), is(0));
+
+        assertThat(deleteIndex(standardIndexName).isAcknowledged(), is(true));
+    }
+
     public void testErrorMessageForLiteralDateMathOverflow() throws IOException {
         List<String> dateMathOverflowExpressions = List.of(
             "2147483647 day + 1 day",
diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvTestsDataLoader.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvTestsDataLoader.java
@@ -77,8 +77,9 @@ public class CsvTestsDataLoader {
     ).withSetting("lookup-settings.json");
     private static final TestDataset LANGUAGES = new TestDataset("languages");
     private static final TestDataset LANGUAGES_LOOKUP = LANGUAGES.withIndex("languages_lookup").withSetting("lookup-settings.json");
-    private static final TestDataset LANGUAGES_LOOKUP_NON_UNIQUE_KEY = LANGUAGES_LOOKUP.withIndex("languages_lookup_non_unique_key")
-        .withData("languages_non_unique_key.csv");
+    private static final TestDataset LANGUAGES_NON_UNIQUE_KEY = new TestDataset("languages_non_unique_key");
+    private static final TestDataset LANGUAGES_LOOKUP_NON_UNIQUE_KEY = LANGUAGES_NON_UNIQUE_KEY.withIndex("languages_lookup_non_unique_key")
+        .withSetting("lookup-settings.json");
     private static final TestDataset LANGUAGES_NESTED_FIELDS = new TestDataset(
         "languages_nested_fields",
         "mapping-languages_nested_fields.json",
diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java
@@ -260,7 +260,11 @@ public static Range rangeOf(Expression value, Expression lower, boolean includeL
     }
 
     public static EsRelation relation() {
-        return new EsRelation(EMPTY, new EsIndex(randomAlphaOfLength(8), emptyMap()), IndexMode.STANDARD);
+        return relation(IndexMode.STANDARD);
+    }
+
+    public static EsRelation relation(IndexMode mode) {
+        return new EsRelation(EMPTY, new EsIndex(randomAlphaOfLength(8), emptyMap()), mode);
     }
 
     /**
diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/inlinestats.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/inlinestats.csv-spec
@@ -3970,3 +3970,23 @@ FROM k8s-downsampled
 2024-05-09T23:30:00.000Z | staging         | three       | {"min":341.0,"max":592.0,"sum":1956.0,"value_count":5}   | 824.0
 2024-05-09T23:30:00.000Z | staging         | two         | {"min":442.0,"max":1011.0,"sum":3850.0,"value_count":7}  | 1419.0
 ;
+
+selfShadowing
+required_capability: inline_stats
+required_capability: fix_join_output_merging
+
+FROM languages_lookup_non_unique_key
+| KEEP country, language_name
+| EVAL language_code = null::integer
+| INLINE STATS MAX(language_code) BY language_code
+| SORT country
+| LIMIT 5
+;
+
+country:text      |language_name:keyword |MAX(language_code):integer |language_code:integer
+Atlantis          |null                  |null                       |null
+[Austria, Germany]|German                |null                       |null
+Canada            |English               |null                       |null
+Mv-Land           |Mv-Lang               |null                       |null
+Mv-Land2          |Mv-Lang2              |null                       |null
+;
diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec
@@ -136,6 +136,22 @@ language_code:integer | language_name:keyword
 6                     |null
 ;
 
+selfShadowing
+required_capability: join_lookup_v12
+required_capability: fix_join_output_merging
+
+FROM languages_lookup_non_unique_key
+| EVAL language_code = null::integer
+| LOOKUP JOIN languages_lookup_non_unique_key ON language_code
+| LIMIT 3
+;
+
+language_code:integer |country:text |country.keyword:keyword |language_name:keyword
+null                  |null         |null                    |null
+null                  |null         |null                    |null
+null                  |null         |null                    |null
+;
+
 nonUniqueLeftKeyOnTheDataNode
 required_capability: join_lookup_v12
 
@@ -240,7 +256,7 @@ emp_no:integer | language_code:integer | language_name:keyword | country:text
     10091      | 1                     | null                  | United Kingdom
     10091      | 1                     | English               | United States of America
     10091      | 1                     | English               | null
-    10092      | 2                     | German                | [Germany, Austria]
+    10092      | 2                     | German                | [Austria, Germany]
     10092      | 2                     | German                | Switzerland
     10092      | 2                     | German                | null
     10093      | 3                     | null                  | null
@@ -265,7 +281,7 @@ emp_no:integer | language_code:integer | language_name:keyword | country:text
 10001          | 1                     | English               | null
 10001          | 1                     | null                  | United Kingdom
 10001          | 1                     | English               | United States of America
-10002          | 2                     | German                | [Germany, Austria]
+10002          | 2                     | German                | [Austria, Germany]
 10002          | 2                     | German                | Switzerland
 10002          | 2                     | German                | null
 10003          | 3                     | null                  | null
@@ -308,7 +324,7 @@ ROW language_code = 2
 
 ignoreOrder:true
 language_code:integer | country:text       | language_name:keyword
-2                     | [Germany, Austria] | German
+2                     | [Austria, Germany] | German
 2                     | Switzerland        | German
 2                     | null               | German
 ;
diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/mapping-languages_non_unique_key.json b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/mapping-languages_non_unique_key.json
@@ -0,0 +1,18 @@
+{
+    "properties" : {
+        "language_code" : {
+            "type" : "integer"
+        },
+        "language_name" : {
+            "type" : "keyword"
+        },
+        "country": {
+            "type": "text",
+            "fields": {
+                "keyword": {
+                    "type": "keyword"
+                }
+            }
+        }
+    }
+}
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java
@@ -1157,6 +1157,11 @@ public enum Cap {
          */
         FIX_JOIN_MASKING_REGEX_EXTRACT,
 
+        /**
+         * Allow the merging of the children to use {@code Aliase}s, instead of just {@code ReferenceAttribute}s.
+         */
+        FIX_JOIN_OUTPUT_MERGING,
+
         /**
          * Avid GROK and DISSECT attributes being removed when resolving fields.
          * see <a href="https://github.com/elastic/elasticsearch/issues/127468"> ES|QL: Grok only supports KEYWORD or TEXT values, found expression [type] type [INTEGER] #127468 </a>
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/local/PruneLeftJoinOnNullMatchingField.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/local/PruneLeftJoinOnNullMatchingField.java
@@ -10,7 +10,6 @@
 import org.elasticsearch.xpack.esql.core.expression.AttributeMap;
 import org.elasticsearch.xpack.esql.core.expression.AttributeSet;
 import org.elasticsearch.xpack.esql.core.expression.Expression;
-import org.elasticsearch.xpack.esql.core.expression.Expressions;
 import org.elasticsearch.xpack.esql.optimizer.LogicalOptimizerContext;
 import org.elasticsearch.xpack.esql.optimizer.rules.RuleUtils;
 import org.elasticsearch.xpack.esql.optimizer.rules.logical.OptimizerRules;
@@ -59,6 +58,6 @@ private static LogicalPlan replaceJoin(Join join) {
         }
         var aliasedNulls = RuleUtils.aliasedNulls(joinRightOutput, a -> true);
         var eval = new Eval(join.source(), join.left(), aliasedNulls.v1());
-        return new Project(join.source(), eval, join.computeOutput(join.left().output(), Expressions.asAttributes(aliasedNulls.v2())));
+        return new Project(join.source(), eval, join.computeOutputExpressions(join.left().output(), aliasedNulls.v2()));
     }
 }
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Lookup.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Lookup.java
@@ -15,6 +15,7 @@
 import org.elasticsearch.xpack.esql.core.capabilities.Resolvables;
 import org.elasticsearch.xpack.esql.core.expression.Attribute;
 import org.elasticsearch.xpack.esql.core.expression.Expression;
+import org.elasticsearch.xpack.esql.core.expression.Expressions;
 import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
 import org.elasticsearch.xpack.esql.core.tree.Source;
 import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
@@ -139,7 +140,7 @@ public List<Attribute> output() {
             if (localRelation == null) {
                 throw new IllegalStateException("Cannot determine output of LOOKUP with unresolved table");
             }
-            lazyOutput = Join.computeOutput(child().output(), localRelation.output(), joinConfig());
+            lazyOutput = Expressions.asAttributes(Join.computeOutputExpressions(child().output(), localRelation.output(), joinConfig()));
         }
         return lazyOutput;
     }
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/InlineJoin.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/InlineJoin.java
@@ -14,7 +14,9 @@
 import org.elasticsearch.compute.data.Page;
 import org.elasticsearch.xpack.esql.core.expression.Alias;
 import org.elasticsearch.xpack.esql.core.expression.Attribute;
+import org.elasticsearch.xpack.esql.core.expression.AttributeMap;
 import org.elasticsearch.xpack.esql.core.expression.Literal;
+import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
 import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
 import org.elasticsearch.xpack.esql.core.tree.Source;
 import org.elasticsearch.xpack.esql.core.util.Holder;
@@ -29,7 +31,7 @@
 import java.util.List;
 import java.util.Set;
 
-import static org.elasticsearch.xpack.esql.expression.NamedExpressions.mergeOutputAttributes;
+import static org.elasticsearch.xpack.esql.expression.NamedExpressions.mergeOutputExpressions;
 import static org.elasticsearch.xpack.esql.plan.logical.join.JoinTypes.LEFT;
 
 /**
@@ -217,16 +219,19 @@ public Join replaceChildren(LogicalPlan left, LogicalPlan right) {
     }
 
     @Override
-    public List<Attribute> computeOutput(List<Attribute> left, List<Attribute> right) {
+    public List<NamedExpression> computeOutputExpressions(List<? extends NamedExpression> left, List<? extends NamedExpression> right) {
         JoinType joinType = config().type();
-        List<Attribute> output;
+        List<NamedExpression> output;
         if (LEFT.equals(joinType)) {
-            List<Attribute> leftOutputWithoutKeys = left.stream().filter(attr -> config().leftFields().contains(attr) == false).toList();
-            List<Attribute> rightWithAppendedKeys = new ArrayList<>(right);
-            rightWithAppendedKeys.removeAll(config().rightFields());
-            rightWithAppendedKeys.addAll(config().leftFields());
-
-            output = mergeOutputAttributes(rightWithAppendedKeys, leftOutputWithoutKeys);
+            List<? extends NamedExpression> leftOutputWithoutKeys = left.stream()
+                .filter(ne -> config().leftFields().contains(ne.toAttribute()) == false)
+                .toList();
+            List<NamedExpression> rightWithAppendedLeftKeys = new ArrayList<>(right);
+            rightWithAppendedLeftKeys.removeIf(ne -> config().rightFields().contains(ne.toAttribute()));
+            AttributeMap<NamedExpression> leftAttrMap = AttributeMap.mapAll(left, NamedExpression::toAttribute);
+            config().leftFields().forEach(lk -> rightWithAppendedLeftKeys.add(leftAttrMap.getOrDefault(lk, lk)));
+
+            output = mergeOutputExpressions(rightWithAppendedLeftKeys, leftOutputWithoutKeys);
         } else {
             throw new IllegalArgumentException(joinType.joinName() + " unsupported");
         }
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/Join.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/Join.java
@@ -18,6 +18,7 @@
 import org.elasticsearch.xpack.esql.core.expression.AttributeSet;
 import org.elasticsearch.xpack.esql.core.expression.Expression;
 import org.elasticsearch.xpack.esql.core.expression.Expressions;
+import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
 import org.elasticsearch.xpack.esql.core.expression.ReferenceAttribute;
 import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
 import org.elasticsearch.xpack.esql.core.tree.Source;
@@ -64,7 +65,7 @@
 import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG;
 import static org.elasticsearch.xpack.esql.core.type.DataType.UNSUPPORTED;
 import static org.elasticsearch.xpack.esql.core.type.DataType.VERSION;
-import static org.elasticsearch.xpack.esql.expression.NamedExpressions.mergeOutputAttributes;
+import static org.elasticsearch.xpack.esql.expression.NamedExpressions.mergeOutputExpressions;
 import static org.elasticsearch.xpack.esql.plan.logical.join.JoinTypes.LEFT;
 import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.commonType;
 
@@ -179,7 +180,7 @@ protected NodeInfo<Join> info() {
     @Override
     public List<Attribute> output() {
         if (lazyOutput == null) {
-            lazyOutput = computeOutput(left().output(), right().output());
+            lazyOutput = Expressions.asAttributes(computeOutputExpressions(left().output(), right().output()));
         }
         return lazyOutput;
     }
@@ -210,30 +211,34 @@ public List<Attribute> rightOutputFields() {
         return rightOutputFields;
     }
 
-    public List<Attribute> computeOutput(List<Attribute> left, List<Attribute> right) {
-        return computeOutput(left, right, config);
+    public List<NamedExpression> computeOutputExpressions(List<? extends NamedExpression> left, List<? extends NamedExpression> right) {
+        return computeOutputExpressions(left, right, config);
     }
 
     /**
      * Combine the two lists of attributes into one.
      * In case of (name) conflicts, specify which sides wins, that is overrides the other column - the left or the right.
      */
-    public static List<Attribute> computeOutput(List<Attribute> leftOutput, List<Attribute> rightOutput, JoinConfig config) {
+    public static List<NamedExpression> computeOutputExpressions(
+        List<? extends NamedExpression> leftOutput,
+        List<? extends NamedExpression> rightOutput,
+        JoinConfig config
+    ) {
         JoinType joinType = config.type();
-        List<Attribute> output;
+        List<NamedExpression> output;
         // TODO: make the other side nullable
         if (LEFT.equals(joinType)) {
             if (config.joinOnConditions() == null) {
                 // right side becomes nullable and overrides left except for join keys, which we preserve from the left
                 AttributeSet rightKeys = AttributeSet.of(config.rightFields());
-                List<Attribute> rightOutputWithoutMatchFields = rightOutput.stream()
-                    .filter(attr -> rightKeys.contains(attr) == false)
+                List<? extends NamedExpression> rightOutputWithoutMatchFields = rightOutput.stream()
+                    .filter(ne -> rightKeys.contains(ne.toAttribute()) == false)
                     .toList();
-                output = mergeOutputAttributes(rightOutputWithoutMatchFields, leftOutput);
+                output = mergeOutputExpressions(rightOutputWithoutMatchFields, leftOutput);
             } else {
                 // We don't allow any attributes in the joinOnConditions that don't have unique names
                 // so right always overwrites left in case of name clashes
-                output = mergeOutputAttributes(rightOutput, leftOutput);
+                output = mergeOutputExpressions(rightOutput, leftOutput);
             }
 
         } else {
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java

Original file line number	Diff line number	Diff line change
`@@ -393,6 +393,12 @@ public static <E> AttributeMap<E> of(Attribute key, E value) {`
`393`	`393`	`return map;`
`394`	`394`	`}`
`395`	`395`
	`396`	`+ public static <E> AttributeMap<E> mapAll(Collection<? extends E> collection, Function<E, Attribute> keyMapper) {`
	`397`	`+ final AttributeMap<E> map = new AttributeMap<>();`
	`398`	`+ collection.forEach(e -> map.add(keyMapper.apply(e), e));`
	`399`	`+ return map;`
	`400`	`+ }`
	`401`	`+`
`396`	`402`	`public static <E> Builder<E> builder() {`
`397`	`403`	`return new Builder<>(new AttributeMap<>());`
`398`	`404`	`}`
Original file line number	Diff line number	Diff line change
`@@ -260,7 +260,11 @@ public static Range rangeOf(Expression value, Expression lower, boolean includeL`
`260`	`260`	`}`
`261`	`261`
`262`	`262`	`public static EsRelation relation() {`
`263`		`- return new EsRelation(EMPTY, new EsIndex(randomAlphaOfLength(8), emptyMap()), IndexMode.STANDARD);`
	`263`	`+ return relation(IndexMode.STANDARD);`
	`264`	`+ }`
	`265`	`+`
	`266`	`+ public static EsRelation relation(IndexMode mode) {`
	`267`	`+ return new EsRelation(EMPTY, new EsIndex(randomAlphaOfLength(8), emptyMap()), mode);`
`264`	`268`	`}`
`265`	`269`
`266`	`270`	`/**`