Ensure stability of clause order for DisjunctionMaxQuery toString (#13944)

Co-authored-by: Laurent <[email protected]>

Author: ljak

lucene/CHANGES.txt

@@ -78,6 +78,7 @@ Bug Fixes
 * GITHUB#13884: Remove broken .toArray from Long/CharObjectHashMap entirely. (Pan Guixin)
 * GITHUB#12686: Added support for highlighting IndexOrDocValuesQuery. (Prudhvi Godithi)
 * GITHUB#13927: Fix StoredFieldsConsumer finish. (linfn)
+* GITHUB#13944: Ensure deterministic order of clauses for `DisjunctionMaxQuery#toString`. (Laurent Jakubina)
 
 Build
 ---------------------

lucene/core/src/java/org/apache/lucene/search/DisjunctionMaxQuery.java

@@ -23,6 +23,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Objects;
+import java.util.stream.Collectors;
 import org.apache.lucene.index.LeafReaderContext;
 
 /**
@@ -44,6 +45,7 @@ public final class DisjunctionMaxQuery extends Query implements Iterable<Query>
 
   /* The subqueries */
   private final Multiset<Query> disjuncts = new Multiset<>();
+  private final List<Query> orderedQueries; // used for toString()
 
   /* Multiple of the non-max disjunct scores added into our final score.  Non-zero values support tie-breaking. */
   private final float tieBreakerMultiplier;
@@ -65,6 +67,7 @@ public DisjunctionMaxQuery(Collection<Query> disjuncts, float tieBreakerMultipli
     }
     this.tieBreakerMultiplier = tieBreakerMultiplier;
     this.disjuncts.addAll(disjuncts);
+    this.orderedQueries = new ArrayList<>(disjuncts); // order from the caller
   }
 
   /**
@@ -295,24 +298,19 @@ public void visit(QueryVisitor visitor) {
    */
   @Override
   public String toString(String field) {
-    StringBuilder buffer = new StringBuilder();
-    buffer.append("(");
-    Iterator<Query> it = disjuncts.iterator();
-    for (int i = 0; it.hasNext(); i++) {
-      Query subquery = it.next();
-      if (subquery instanceof BooleanQuery) { // wrap sub-bools in parens
-        buffer.append("(");
-        buffer.append(subquery.toString(field));
-        buffer.append(")");
-      } else buffer.append(subquery.toString(field));
-      if (i != disjuncts.size() - 1) buffer.append(" | ");
-    }
-    buffer.append(")");
-    if (tieBreakerMultiplier != 0.0f) {
-      buffer.append("~");
-      buffer.append(tieBreakerMultiplier);
-    }
-    return buffer.toString();
+    return this.orderedQueries.stream()
+        .map(
+            subquery -> {
+              if (subquery instanceof BooleanQuery) { // wrap sub-bools in parens
+                return "(" + subquery.toString(field) + ")";
+              }
+              return subquery.toString(field);
+            })
+        .collect(
+            Collectors.joining(
+                " | ",
+                "(",
+                ")" + ((tieBreakerMultiplier != 0.0f) ? "~" + tieBreakerMultiplier : "")));
   }
 
   /**

lucene/core/src/test/org/apache/lucene/search/TestDisjunctionMaxQuery.java

@@ -25,6 +25,7 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
+import java.util.stream.Collectors;
 import org.apache.lucene.analysis.standard.StandardAnalyzer;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.Field;
@@ -489,6 +490,27 @@ public void testDisjunctOrderAndEquals() throws Exception {
     assertEquals(q1, q2);
   }
 
+  /* Inspired from TestIntervals.testIntervalDisjunctionToStringStability */
+  public void testToStringOrderMatters() {
+    final int clauseNbr =
+        random().nextInt(22) + 4; // ensure a reasonably large minimum number of clauses
+    final String[] terms = new String[clauseNbr];
+    for (int i = 0; i < clauseNbr; i++) {
+      terms[i] = Character.toString((char) ('a' + i));
+    }
+
+    final String expected =
+        Arrays.stream(terms)
+            .map((term) -> "test:" + term)
+            .collect(Collectors.joining(" | ", "(", ")~1.0"));
+
+    DisjunctionMaxQuery source =
+        new DisjunctionMaxQuery(
+            Arrays.stream(terms).map((term) -> tq("test", term)).toList(), 1.0f);
+
+    assertEquals(expected, source.toString(""));
+  }
+
   public void testRandomTopDocs() throws Exception {
     doTestRandomTopDocs(2, 0.05f, 0.05f);
     doTestRandomTopDocs(2, 1.0f, 0.05f);