Serialization changes for JoinConfig, comments

Author: julian-elastic

x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join-expression.csv-spec

@@ -710,7 +710,7 @@ emp_no:integer | language_code_left:integer | language_name:keyword
 
 lookupJoinExpressionWithPushableFilterOnRight
 required_capability: join_lookup_v12
-required_capability: lookup_join_on_multiple_fields
+required_capability: lookup_join_on_boolean_expression
 
 FROM multi_column_joinable
 | RENAME id_int AS id_left, is_active_bool AS is_active_left

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

@@ -28,6 +28,18 @@
 import java.util.List;
 import java.util.function.IntFunction;
 
+/**
+ * A {@link QueryList} that generates a query for a binary comparison.
+ * This class is used in the context of an expression based lookup join,
+ * where we need to generate a query for each row of the left dataset.
+ * The query is then used to fetch the matching rows from the right dataset.
+ * The query is a binary comparison between a field from the right dataset and a value from the left dataset.
+ * The field is on the left side of the comparison and the value is on the right side.
+ * The value is extracted from a block at a given position.
+ * The comparison is then translated to a Lucene query.
+ * If the comparison cannot be translated, an exception is thrown.
+ * This class is used in conjunction with {@link ExpressionQueryList} to generate the final query for the lookup join.
+ */
 public class BinaryComparisonQueryList extends QueryList {
     private final EsqlBinaryComparison binaryComparison;
     private final IntFunction<Object> blockValueReader;

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

@@ -44,6 +44,11 @@
  * Each query in the resulting query will be a conjunction of all queries from the input lists at the same position.
  * In addition, we support an optional pre-join filter that will be applied to all queries if it is pushable.
  * If the pre-join filter cannot be pushed down to Lucene, it will be ignored.
+ * This class is used in the context of a lookup join, where we need to generate a query for each row of the left dataset.
+ * The query is then used to fetch the matching rows from the right dataset.
+ * The class supports two types of joins:
+ * 1. Field-based join: The join conditions are based on the equality of fields from the left and right datasets.
+ * 2. Expression-based join: The join conditions are based on a complex expression that can involve multiple fields and operators.
  */
 public class ExpressionQueryList implements LookupEnrichQueryGenerator {
     private final List<QueryList> queryLists;
@@ -64,6 +69,20 @@ private ExpressionQueryList(
         buildPreJoinFilter(rightPreJoinPlan, clusterService);
     }
 
+    /**
+     * 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.
+     * For example | LOOKUP JOIN on field1, field2, field3
+     * The query lists are generated from the join conditions.
+     * The pre-join filter is an optional filter that is applied to the right dataset before the join.
+     * @param queryLists The list of query lists that will be combined.
+     * @param context The search execution context.
+     * @param rightPreJoinPlan The physical plan for the right side of the join.
+     * @param clusterService The cluster service.
+     * @param aliasFilter The alias filter.
+     * @return A new {@link ExpressionQueryList} for a field-based join.
+     * @throws IllegalArgumentException if the number of query lists is less than 2 and there is no pre-join filter.
+     */
     public static ExpressionQueryList fieldBasedJoin(
         List<QueryList> queryLists,
         SearchExecutionContext context,
@@ -77,6 +96,22 @@ public static ExpressionQueryList fieldBasedJoin(
         return new ExpressionQueryList(queryLists, context, rightPreJoinPlan, clusterService, aliasFilter);
     }
 
+    /**
+     * Creates a new {@link ExpressionQueryList} for an expression-based join.
+     * An expression-based join is a join where the join conditions are based on a complex expression
+     * that can involve multiple fields and operators.
+     * Example | LOOKUP JOIN on left_field > right_field AND left_field2 == right_field2
+     * The query lists are generated from the join conditions.
+     * The pre-join filter is an optional filter that is applied to the right dataset before the join.
+     * @param context The search execution context.
+     * @param rightPreJoinPlan The physical plan for the right side of the join.
+     * @param clusterService The cluster service.
+     * @param request The transport request.
+     * @param aliasFilter The alias filter.
+     * @param warnings The warnings.
+     * @return A new {@link ExpressionQueryList} for an expression-based join.
+     * @throws IllegalStateException if the join conditions are null.
+     */
     public static ExpressionQueryList expressionBasedJoin(
         SearchExecutionContext context,
         PhysicalPlan rightPreJoinPlan,
@@ -207,6 +242,13 @@ private void buildPreJoinFilter(PhysicalPlan rightPreJoinPlan, ClusterService cl
         }
     }
 
+    /**
+     * Returns the query at the given position.
+     * The query is a conjunction of all queries from the input lists at the same position.
+     * If a pre-join filter exists, it is also added to the query.
+     * @param position The position of the query to return.
+     * @return The query at the given position, or null if any of the match fields are null.
+     */
     @Override
     public Query getQuery(int position) {
         BooleanQuery.Builder builder = new BooleanQuery.Builder();
@@ -226,6 +268,12 @@ public Query getQuery(int position) {
         return builder.build();
     }
 
+    /**
+     * Returns the number of positions in the query list.
+     * The number of positions is the same for all query lists.
+     * @return The number of positions in the query list.
+     * @throws IllegalArgumentException if the query lists have different position counts.
+     */
     @Override
     public int getPositionCount() {
         int positionCount = queryLists.get(0).getPositionCount();

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/JoinConfig.java

@@ -30,31 +30,17 @@ public record JoinConfig(JoinType type, List<Attribute> leftFields, List<Attribu
     implements
         Writeable {
 
-    /**
-     * Legacy constructor that included the match fields, which were always the left fields.
-     * They are kept here for serialization compatibility, but are not used anymore.
-     */
-    // TODO: Remove
-    @Deprecated(forRemoval = true)
-    private JoinConfig(
-        JoinType type,
-        List<Attribute> matchFields,
-        List<Attribute> leftFields,
-        List<Attribute> rightFields,
-        Expression joinOnConditions
-    ) {
-        this(type, leftFields, rightFields, joinOnConditions);
+    public JoinConfig(StreamInput in) throws IOException {
+        this(JoinTypes.readFrom(in), readLeftFields(in), in.readNamedWriteableCollectionAsList(Attribute.class), readJoinConditions(in));
     }
 
-    public JoinConfig(StreamInput in) throws IOException {
-        this(
-            JoinTypes.readFrom(in),
-            // TODO we read the match fields for legacy reasons, they are not used anymore.
-            in.readNamedWriteableCollectionAsList(Attribute.class),
-            in.readNamedWriteableCollectionAsList(Attribute.class),
-            in.readNamedWriteableCollectionAsList(Attribute.class),
-            readJoinConditions(in)
-        );
+    private static List<Attribute> readLeftFields(StreamInput in) throws IOException {
+        if (in.getTransportVersion().onOrAfter(TransportVersions.ESQL_LOOKUP_JOIN_ON_EXPRESSION) == false) {
+            // For BWC, the left fields were written twice (once as match fields)
+            // We read the first set and ignore them.
+            in.readNamedWriteableCollectionAsList(Attribute.class);
+        }
+        return in.readNamedWriteableCollectionAsList(Attribute.class);
     }
 
     private static Expression readJoinConditions(StreamInput in) throws IOException {
@@ -67,8 +53,9 @@ private static Expression readJoinConditions(StreamInput in) throws IOException
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         type.writeTo(out);
-        // TODO we write the match fields for legacy reasons, they used to always be the left fields.
-        out.writeNamedWriteableCollection(leftFields);
+        if (out.getTransportVersion().onOrAfter(TransportVersions.ESQL_LOOKUP_JOIN_ON_EXPRESSION) == false) {
+            out.writeNamedWriteableCollection(leftFields);
+        }
         out.writeNamedWriteableCollection(leftFields);
         out.writeNamedWriteableCollection(rightFields);
         if (out.getTransportVersion().onOrAfter(TransportVersions.ESQL_LOOKUP_JOIN_ON_EXPRESSION)) {