Address more code review comments

Author: julian-elastic

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

@@ -111,7 +111,7 @@ protected LookupEnrichQueryGenerator queryList(
         Block inputBlock,
         Warnings warnings
     ) {
-        PhysicalPlan physicalPlan = localLookupNodePlanning(request.rightPreJoinPlan);
+        PhysicalPlan lookupNodePlan = localLookupNodePlanning(request.rightPreJoinPlan);
         if (request.joinOnConditions == null) {
             // this is a field based join
             List<QueryList> queryLists = new ArrayList<>();
@@ -126,13 +126,13 @@ protected LookupEnrichQueryGenerator queryList(
                 ).onlySingleValues(warnings, "LOOKUP JOIN encountered multi-value");
                 queryLists.add(q);
             }
-            if (queryLists.size() == 1 && physicalPlan instanceof FilterExec == false) {
+            if (queryLists.size() == 1 && lookupNodePlan instanceof FilterExec == false) {
                 return queryLists.getFirst();
             }
-            return ExpressionQueryList.fieldBasedJoin(queryLists, context, physicalPlan, clusterService, aliasFilter);
+            return ExpressionQueryList.fieldBasedJoin(queryLists, context, lookupNodePlan, clusterService, aliasFilter);
         } else {
             // this is an expression based join
-            return ExpressionQueryList.expressionBasedJoin(context, physicalPlan, clusterService, request, aliasFilter, warnings);
+            return ExpressionQueryList.expressionBasedJoin(context, lookupNodePlan, clusterService, request, aliasFilter, warnings);
         }
 
     }

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

@@ -16,6 +16,24 @@
 import java.io.IOException;
 import java.util.Objects;
 
+/**
+ * Configuration for a field used in the join condition of a LOOKUP JOIN or ENRICH operation.
+ * <p>
+ * This class specifies how to match a field from the input data (the "left" side of the join)
+ * with a field in the lookup index (the "right" side). The interpretation of its properties
+ * depends on the type of join.
+ * <p>
+ * For simple field-based joins (e.g., {@code ... ON field1, field2}), this configuration
+ * represents the right-side field ({@code right.field}). In this case, {@link #fieldName} is the
+ * name of the field in the lookup index used to build the query.
+ * <p>
+ * For expression-based joins (e.g., {@code ... ON left_field > right_field}), this
+ * configuration represents the left-side field ({@code left_field}). In this case,
+ * {@link #fieldName} is the name of the field whose value is sent to the lookup node.
+ * <p>
+ * The {@link #channel} identifies the position of this field's values within the internal
+ * page sent to the lookup node.
+ */
 public final class MatchConfig implements Writeable {
     private final String fieldName;
     private final int channel;

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java

@@ -2509,6 +2509,33 @@ public void testPruneJoinOnNullMatchingField() {
         var source = as(limit.child(), EsRelation.class);
     }
 
+    /**
+     * <pre>{@code
+     * Project[[emp_no{f}#10, first_name{f}#11, languages{f}#13, language_code_left{r}#3, language_code{r}#21, language_name{
+     * r}#22]]
+     * \_Eval[[null[INTEGER] AS language_code_left#3, null[INTEGER] AS language_code#21, null[KEYWORD] AS language_name#22]]
+     *   \_Limit[1000[INTEGER],false]
+     *     \_EsRelation[test][_meta_field{f}#16, emp_no{f}#10, first_name{f}#11, ..]
+     * }</pre>
+     */
+    public void testPruneJoinOnNullMatchingFieldExpressionJoin() {
+        var plan = optimizedPlan("""
+            from test
+            | eval language_code_left = null::integer
+            | keep emp_no, first_name, languages, language_code_left
+            | lookup join languages_lookup on language_code_left == language_code
+            """);
+
+        var project = as(plan, Project.class);
+        assertThat(
+            Expressions.names(project.output()),
+            contains("emp_no", "first_name", "languages", "language_code_left", "language_code", "language_name")
+        );
+        var eval = as(project.child(), Eval.class);
+        var limit = asLimit(eval.child(), 1000, false);
+        var source = as(limit.child(), EsRelation.class);
+    }
+
     /**
      * <pre>{@code
      * Project[[emp_no{f}#15, first_name{f}#16, my_null{r}#3 AS language_code#9, language_name{r}#27]]
@@ -2534,6 +2561,31 @@ public void testPruneJoinOnNullAssignedMatchingField() {
         var source = as(limit.child(), EsRelation.class);
     }
 
+    /**
+     * <pre>{@code
+     * Project[[emp_no{f}#16, first_name{f}#17, language_code{r}#27, language_name{r}#28]]
+     * \_Eval[[null[INTEGER] AS language_code#27, null[KEYWORD] AS language_name#28]]
+     *   \_Limit[1000[INTEGER],false]
+     *     \_EsRelation[test][_meta_field{f}#22, emp_no{f}#16, first_name{f}#17, ..]
+     * }</pre>
+     */
+    public void testPruneJoinOnNullAssignedMatchingFieldExpr() {
+        var plan = optimizedPlan("""
+            from test
+            | eval my_null = null::integer
+            | rename languages as language_code_right
+            | eval language_code_right = my_null
+            | lookup join languages_lookup on language_code_right > language_code
+            | keep emp_no, first_name, language_code, language_name
+            """);
+
+        var project = as(plan, Project.class);
+        assertThat(Expressions.names(project.output()), contains("emp_no", "first_name", "language_code", "language_name"));
+        var eval = as(project.child(), Eval.class);
+        var limit = asLimit(eval.child(), 1000, false);
+        var source = as(limit.child(), EsRelation.class);
+    }
+
     private static List<String> orderNames(TopN topN) {
         return topN.order().stream().map(o -> as(o.child(), NamedExpression.class).name()).toList();
     }

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/physical/LookupJoinExecSerializationTests.java

@@ -74,19 +74,12 @@ protected LookupJoinExec mutateInstance(LookupJoinExec instance) throws IOExcept
             case 2 -> leftFields = randomValueOtherThan(leftFields, LookupJoinExecSerializationTests::randomFields);
             case 3 -> rightFields = randomValueOtherThan(rightFields, LookupJoinExecSerializationTests::randomFields);
             case 4 -> addedFields = randomValueOtherThan(addedFields, LookupJoinExecSerializationTests::randomFields);
-            case 5 -> joinOnConditions = mutateJoinOnCondition(joinOnConditions);
+            case 5 -> joinOnConditions = randomValueOtherThan(joinOnConditions, LookupJoinExecSerializationTests::randomJoinOnExpression);
             default -> throw new UnsupportedOperationException();
         }
         return new LookupJoinExec(instance.source(), child, lookup, leftFields, rightFields, addedFields, joinOnConditions);
     }
 
-    private Expression mutateJoinOnCondition(Expression joinOnConditions) {
-        if (joinOnConditions != null) {
-            return null;
-        }
-        return randomJoinOnExpression();
-    }
-
     @Override
     protected boolean alwaysEmptySource() {
         return true;