Bugfix

Author: julian-elastic

x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/LookupJoinGeneralExpressionIT.java

@@ -224,7 +224,7 @@ private void loadSingleDataset(
     }
 
     private EsqlQueryResponse runQuery(String query) {
-        return run(syncEsqlQueryRequest().query(query));
+        return run(syncEsqlQueryRequest(query));
     }
 
     /**

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/AbstractLookupService.java

@@ -79,13 +79,15 @@
 import org.elasticsearch.xpack.esql.core.expression.Expression;
 import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
 import org.elasticsearch.xpack.esql.core.expression.FoldContext;
+import org.elasticsearch.xpack.esql.core.expression.NameId;
 import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
-import org.elasticsearch.xpack.esql.core.expression.TypedAttribute;
 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.evaluator.EvalMapper;
 import org.elasticsearch.xpack.esql.expression.predicate.Predicates;
+import org.elasticsearch.xpack.esql.plan.logical.EsRelation;
+import org.elasticsearch.xpack.esql.plan.physical.FragmentExec;
 import org.elasticsearch.xpack.esql.planner.EsPhysicalOperationProviders;
 import org.elasticsearch.xpack.esql.planner.Layout;
 import org.elasticsearch.xpack.esql.planner.PlannerUtils;
@@ -94,7 +96,6 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -586,38 +587,46 @@ private List<MatchConfig> collectLeftSideFieldsToBroadcast(LookupEnrichQueryGene
             List<Expression> postJoinFilterExpressions = postJoinFilterable.getPostJoinFilter();
             if (postJoinFilterExpressions.isEmpty() == false) {
                 LookupFromIndexService.TransportRequest lookupRequest = (LookupFromIndexService.TransportRequest) request;
-                // Build a map of matchField names for quick lookup
-                Map<String, MatchConfig> matchFieldsByName = new HashMap<>();
-                for (MatchConfig matchField : lookupRequest.getMatchFields()) {
-                    matchFieldsByName.put(matchField.fieldName(), matchField);
-                }
-                // Build a set of extractFields names to avoid adding duplicates
-                Set<String> extractFieldNames = new HashSet<>();
+                // Build a set of extractFields NameIDs to avoid adding duplicates
+                Set<NameId> extractFieldNameIds = new HashSet<>();
                 for (NamedExpression extractField : request.extractFields) {
-                    extractFieldNames.add(extractField.name());
+                    extractFieldNameIds.add(extractField.id());
+                }
+                // Collect right-side field NameIDs from EsRelation in rightPreJoinPlan
+                Set<NameId> rightSideFieldNameIds = new HashSet<>();
+                if (lookupRequest.getRightPreJoinPlan() instanceof FragmentExec fragmentExec) {
+                    fragmentExec.fragment().forEachDown(EsRelation.class, esRelation -> {
+                        for (Attribute attr : esRelation.output()) {
+                            rightSideFieldNameIds.add(attr.id());
+                        }
+                    });
                 }
-                // Extract FieldAttributes from post-join filter expressions and check if they are in matchFields
-                Map<String, TypedAttribute> fieldsToBroadcast = new HashMap<>();
+
+                // Track which NameIDs we've already added to avoid duplicates
+                Set<NameId> addedNameIds = new HashSet<>();
+                // Traverse filter expressions and match attributes to MatchConfigs by NameID
+                // Exclude right-side fields and fields already in extractFields
                 for (Expression filterExpr : postJoinFilterExpressions) {
                     for (Attribute attr : filterExpr.references()) {
-                        if (attr instanceof TypedAttribute typedAttribute) {
-                            String fieldName = attr.name();
-                            // Check if this field is in matchFields (left-side field with a channel)
-                            if (matchFieldsByName.containsKey(fieldName)) {
-                                fieldsToBroadcast.put(fieldName, typedAttribute);
+                        NameId nameId = attr.id();
+                        // Skip if right-side field, already in extractFields, or already found a match for
+                        if (rightSideFieldNameIds.contains(nameId)
+                            || extractFieldNameIds.contains(nameId)
+                            || addedNameIds.contains(nameId)) {
+                            continue;
+                        }
+                        // Find the corresponding MatchConfig for this attribute
+                        // we do match by just name
+                        // we made sure the same attribute is not on the right side with the checks above
+                        for (MatchConfig matchField : lookupRequest.getMatchFields()) {
+                            if (attr.name().equals(matchField.fieldName())) {
+                                builder.append(attr);
+                                allLeftFieldsToBroadcast.add(matchField);
+                                addedNameIds.add(nameId);
+                                break;
                             }
                         }
-                    }
-                }
-                // Add matching fields to layout and broadcast list, but skip if already in extractFields
-                for (Map.Entry<String, TypedAttribute> entry : fieldsToBroadcast.entrySet()) {
-                    String fieldName = entry.getKey();
-                    TypedAttribute typedAttribute = entry.getValue();
-                    MatchConfig matchField = matchFieldsByName.get(fieldName);
-                    // Only add if not already in extractFields to avoid duplicates
-                    if (extractFieldNames.contains(fieldName) == false) {
-                        builder.append(typedAttribute);
-                        allLeftFieldsToBroadcast.add(matchField);
+
                     }
                 }
             }

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/ExpressionQueryList.java

@@ -25,6 +25,7 @@
 import org.elasticsearch.xpack.esql.capabilities.TranslationAware;
 import org.elasticsearch.xpack.esql.core.expression.Attribute;
 import org.elasticsearch.xpack.esql.core.expression.Expression;
+import org.elasticsearch.xpack.esql.core.expression.NameId;
 import org.elasticsearch.xpack.esql.core.type.DataType;
 import org.elasticsearch.xpack.esql.expression.predicate.Predicates;
 import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.Equals;
@@ -65,6 +66,7 @@ public class ExpressionQueryList implements LookupEnrichQueryGenerator, PostJoin
     private final SearchExecutionContext context;
     private final AliasFilter aliasFilter;
     private final LucenePushdownPredicates lucenePushdownPredicates;
+    private final Set<NameId> rightSideFieldNameIds;
     private List<Expression> postJoinFilter;
     private int inputPagePositionCount = -1;
 
@@ -82,10 +84,23 @@ private ExpressionQueryList(
             SearchContextStats.from(List.of(context)),
             new EsqlFlags(clusterService.getClusterSettings())
         );
+        this.rightSideFieldNameIds = collectRightSideFieldNameIds(rightPreJoinPlan);
         postJoinFilter = new ArrayList<>();
         buildPreJoinFilter(rightPreJoinPlan, clusterService);
     }
 
+    private static Set<NameId> collectRightSideFieldNameIds(PhysicalPlan rightPreJoinPlan) {
+        Set<NameId> rightSideFieldNameIds = new HashSet<>();
+        if (rightPreJoinPlan != null) {
+            rightPreJoinPlan.forEachDown(EsSourceExec.class, esSourceExec -> {
+                for (Attribute attr : esSourceExec.output()) {
+                    rightSideFieldNameIds.add(attr.id());
+                }
+            });
+        }
+        return rightSideFieldNameIds;
+    }
+
     /**
      * Creates a new {@link ExpressionQueryList} for a field-based join.
      * A field-based join is a join where the join conditions are based on the equality of fields from the left and right datasets.
@@ -155,17 +170,12 @@ private void buildJoinOnForExpressionJoin(
         this.inputPagePositionCount = inputPage.getPositionCount();
         List<Expression> expressions = Predicates.splitAnd(joinOnConditions);
 
-        // Build set of left-side field names for categorization
-        Set<String> leftSideFieldNames = new HashSet<>();
-        for (MatchConfig matchField : matchFields) {
-            leftSideFieldNames.add(matchField.fieldName());
-        }
-
         // Split expressions into left-only, right-only, and mixed
+        // Anything not in right-side fields is left-side
         List<Expression> leftOnlyExpressions = new ArrayList<>();
         List<Expression> rightOnlyExpressions = new ArrayList<>();
         List<Expression> mixedExpressions = new ArrayList<>();
-        splitExpressionsBySide(expressions, leftSideFieldNames, leftOnlyExpressions, rightOnlyExpressions, mixedExpressions);
+        splitExpressionsBySide(expressions, rightSideFieldNameIds, leftOnlyExpressions, rightOnlyExpressions, mixedExpressions);
 
         // Process mixed expressions - try as left-right binary comparison first
         // If that fails, add to post-join filter
@@ -191,7 +201,7 @@ private void buildJoinOnForExpressionJoin(
 
     private void splitExpressionsBySide(
         List<Expression> expressions,
-        Set<String> leftSideFieldNames,
+        Set<NameId> rightSideFieldNameIds,
         List<Expression> leftOnlyExpressions,
         List<Expression> rightOnlyExpressions,
         List<Expression> mixedExpressions
@@ -204,10 +214,11 @@ private void splitExpressionsBySide(
             boolean hasRightSide = false;
 
             for (Attribute attr : allAttributes) {
-                if (leftSideFieldNames.contains(attr.name())) {
-                    hasLeftSide = true;
-                } else {
+                NameId nameId = attr.id();
+                if (rightSideFieldNameIds.contains(nameId)) {
                     hasRightSide = true;
+                } else {
+                    hasLeftSide = true;
                 }
             }
 
@@ -222,19 +233,13 @@ private void splitExpressionsBySide(
     }
 
     private boolean applyAsRightSidePushableFilter(Expression filter, List<MatchConfig> matchFields) {
-        // First check if this filter only references right-side attributes
-        // Right-side attributes are those NOT in matchFields (which are left-side fields)
-        Set<String> leftSideFieldNames = new HashSet<>();
-        for (MatchConfig matchField : matchFields) {
-            leftSideFieldNames.add(matchField.fieldName());
-        }
         // Check if any attribute in the filter expression tree is from the left side
         // We need to traverse the entire expression tree, not just top-level references,
         // because some functions may have attributes nested in their children
         List<Attribute> allAttributes = new ArrayList<>();
         filter.forEachDown(Attribute.class, allAttributes::add);
         for (Attribute attr : allAttributes) {
-            if (leftSideFieldNames.contains(attr.name())) {
+            if (rightSideFieldNameIds.contains(attr.id()) == false) {
                 // This filter references a left-side attribute, so it cannot be pushed to Lucene
                 return false;
             }
@@ -257,20 +262,58 @@ private boolean applyAsRightSidePushableFilter(Expression filter, List<MatchConf
         return false;
     }
 
+    /**
+     * Reorients a binary comparison so that the left side is from the input page and the right side is from the lookup index.
+     * Returns the comparison as-is if already correctly oriented, or a swapped version if needed.
+     * Returns null if both attributes are from the same side (can't be reoriented).
+     */
+    private static EsqlBinaryComparison reorientBinaryComparison(EsqlBinaryComparison binaryComparison, Set<NameId> rightSideFieldNameIds) {
+        if (binaryComparison.left() instanceof Attribute leftAttr && binaryComparison.right() instanceof Attribute rightAttr) {
+            // Determine which attribute is from the right side (lookup index)
+            boolean leftIsRightSide = rightSideFieldNameIds.contains(leftAttr.id());
+            boolean rightIsRightSide = rightSideFieldNameIds.contains(rightAttr.id());
+
+            // We need exactly one attribute from the right side and one from the left side
+            if (leftIsRightSide == rightIsRightSide) {
+                // Both are from the same side, can't process as left-right comparison
+                return null;
+            }
+
+            if (rightIsRightSide) {
+                // Original orientation is correct: left is from input, right is from lookup
+                return binaryComparison;
+            } else {
+                // Need to swap: original left is from lookup, original right is from input
+                // Swap the comparison and flip the operator if needed
+                return (EsqlBinaryComparison) binaryComparison.swapLeftAndRight();
+            }
+        }
+        return null;
+    }
+
     private boolean applyAsLeftRightBinaryComparison(
         Expression expr,
         List<MatchConfig> matchFields,
         Page inputPage,
         ClusterService clusterService,
         Warnings warnings
     ) {
-        if (expr instanceof EsqlBinaryComparison binaryComparison
-            && binaryComparison.left() instanceof Attribute leftAttribute
-            && binaryComparison.right() instanceof Attribute rightAttribute) {
-            // the left side comes from the page that was sent to the lookup node
-            // the right side is the field from the lookup index
-            // check if the left side is in the matchFields
-            // if it is its corresponding page is the corresponding number in inputPage
+        if (expr instanceof EsqlBinaryComparison binaryComparison) {
+            // Reorient the comparison so that left is from input page and right is from lookup index
+            EsqlBinaryComparison orientedComparison = reorientBinaryComparison(binaryComparison, rightSideFieldNameIds);
+            if (orientedComparison == null) {
+                // Can't reorient (both attributes from same side)
+                return false;
+            }
+
+            // After reorientation, left is from input page and right is from lookup index
+            Attribute leftAttribute = (Attribute) orientedComparison.left();
+            Attribute rightAttribute = (Attribute) orientedComparison.right();
+
+            // The left side comes from the page that was sent to the lookup node
+            // The right side is the field from the lookup index
+            // Check if the left side is in the matchFields
+            // If it is its corresponding page is the corresponding number in inputPage
             Block block = null;
             DataType dataType = null;
             for (int i = 0; i < matchFields.size(); i++) {
@@ -285,7 +328,7 @@ private boolean applyAsLeftRightBinaryComparison(
                 // special handle Equals operator
                 // TermQuery is faster than BinaryComparisonQueryList, as it does less work per row
                 // so here we reuse the existing logic from field based join to build a termQueryList for Equals
-                if (binaryComparison instanceof Equals) {
+                if (orientedComparison instanceof Equals) {
                     QueryList termQueryForEquals = termQueryList(rightFieldType, context, aliasFilter, block, dataType).onlySingleValues(
                         warnings,
                         "LOOKUP JOIN encountered multi-value"
@@ -297,7 +340,7 @@ private boolean applyAsLeftRightBinaryComparison(
                             rightFieldType,
                             context,
                             block,
-                            binaryComparison,
+                            orientedComparison,
                             clusterService,
                             aliasFilter,
                             warnings

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java

@@ -291,6 +291,10 @@ public List<MatchConfig> getMatchFields() {
             return matchFields;
         }
 
+        public PhysicalPlan getRightPreJoinPlan() {
+            return rightPreJoinPlan;
+        }
+
         @Override
         public void writeTo(StreamOutput out) throws IOException {
             super.writeTo(out);