WritableQueryIndex Relies On Hashcode Invariance Across JVMs

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

@@ -85,6 +85,13 @@ public Collection<Query> getDisjuncts() {
     return Collections.unmodifiableCollection(disjuncts);
   }
 
+  /**
+   * @return the disjuncts in the same order as the input Query collection.
+   */
+  public Collection<Query> getDisjunctsInOriginalOrder() {
+    return Collections.unmodifiableCollection(orderedQueries);
+  }
+
   /**
    * @return tie breaker value for multiple matches.
    */

lucene/monitor/src/java/org/apache/lucene/monitor/QueryDecomposer.java

@@ -19,13 +19,13 @@
 
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.Set;
 import org.apache.lucene.search.BooleanClause;
 import org.apache.lucene.search.BooleanQuery;
 import org.apache.lucene.search.BoostQuery;
 import org.apache.lucene.search.DisjunctionMaxQuery;
 import org.apache.lucene.search.Query;
-import org.apache.lucene.util.CollectionUtil;
 
 /**
  * Split a disjunction query into its consituent parts, so that they can be indexed and run
@@ -44,8 +44,8 @@ public Set<Query> decompose(Query q) {
     if (q instanceof BooleanQuery) return decomposeBoolean((BooleanQuery) q);
 
     if (q instanceof DisjunctionMaxQuery) {
-      Set<Query> subqueries = new HashSet<>();
-      for (Query subq : ((DisjunctionMaxQuery) q).getDisjuncts()) {
+      Set<Query> subqueries = new LinkedHashSet<>();
+      for (Query subq : ((DisjunctionMaxQuery) q).getDisjunctsInOriginalOrder()) {
         subqueries.addAll(decompose(subq));
       }
       return subqueries;
@@ -61,7 +61,7 @@ public Set<Query> decompose(Query q) {
   public Set<Query> decomposeBoostQuery(BoostQuery q) {
     if (q.getBoost() == 1.0) return decompose(q.getQuery());
 
-    Set<Query> boostedDecomposedQueries = new HashSet<>();
+    Set<Query> boostedDecomposedQueries = new LinkedHashSet<>();
     for (Query subq : decompose(q.getQuery())) {
       boostedDecomposedQueries.add(new BoostQuery(subq, q.getBoost()));
     }
@@ -77,7 +77,7 @@ public Set<Query> decomposeBoostQuery(BoostQuery q) {
   public Set<Query> decomposeBoolean(BooleanQuery q) {
     if (q.getMinimumNumberShouldMatch() > 1) return Collections.singleton(q);
 
-    Set<Query> subqueries = new HashSet<>();
+    Set<Query> subqueries = new LinkedHashSet<>();
     Set<Query> exclusions = new HashSet<>();
     Set<Query> mandatory = new HashSet<>();
 
@@ -104,7 +104,7 @@ public Set<Query> decomposeBoolean(BooleanQuery q) {
 
     // If there are exclusions, then we need to add them to all the decomposed
     // queries
-    Set<Query> rewrittenSubqueries = CollectionUtil.newHashSet(subqueries.size());
+    Set<Query> rewrittenSubqueries = new LinkedHashSet<>();
     for (Query subquery : subqueries) {
       BooleanQuery.Builder bq = new BooleanQuery.Builder();
       bq.add(subquery, BooleanClause.Occur.MUST);

lucene/monitor/src/test/org/apache/lucene/monitor/TestMonitorPersistence.java

@@ -20,19 +20,32 @@
 import java.io.IOException;
 import java.nio.file.Path;
 import java.util.Collections;
+import java.util.Objects;
+import java.util.function.Function;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.Field;
+import org.apache.lucene.search.BooleanClause;
+import org.apache.lucene.search.BooleanQuery;
+import org.apache.lucene.search.IndexSearcher;
 import org.apache.lucene.search.MatchAllDocsQuery;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.QueryVisitor;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.Weight;
+import org.apache.lucene.util.StringHelper;
 
 public class TestMonitorPersistence extends MonitorTestBase {
 
   private Path indexDirectory = createTempDir();
 
   protected Monitor newMonitorWithPersistence() throws IOException {
+    return newMonitorWithPersistence(MonitorTestBase::parse);
+  }
+
+  protected Monitor newMonitorWithPersistence(Function<String, Query> parser) throws IOException {
     MonitorConfiguration config =
         new MonitorConfiguration()
-            .setIndexPath(
-                indexDirectory, MonitorQuerySerializer.fromParser(MonitorTestBase::parse));
+            .setIndexPath(indexDirectory, MonitorQuerySerializer.fromParser(parser));
 
     return new Monitor(ANALYZER, config);
   }
@@ -96,4 +109,88 @@ public void testEphemeralMonitorDoesNotStoreQueries() throws IOException {
           "Cannot get queries from an index with no MonitorQuerySerializer", e.getMessage());
     }
   }
+
+  public void testReadingAfterBreakingUpgrade() throws IOException {
+    Document doc = new Document();
+    doc.add(newTextField(FIELD, "test", Field.Store.NO));
+    Function<String, Query> parser =
+        queryStr -> {
+          var query = (BooleanQuery) MonitorTestBase.parse(queryStr);
+          return incompatibleBooleanQuery(query);
+        };
+    try (Monitor monitor = newMonitorWithPersistence(parser)) {
+      StringBuilder queryStr = new StringBuilder();
+      for (int i = 0; i < 100; ++i) {
+        queryStr.append("test").append(i).append(" OR ");
+      }
+      queryStr.append(" test");
+      var mq =
+          new MonitorQuery(
+              "1",
+              incompatibleBooleanQuery((BooleanQuery) parse(queryStr.toString())),
+              queryStr.toString(),
+              Collections.emptyMap());
+      monitor.register(mq);
+      assertEquals(1, monitor.getQueryCount());
+      assertEquals(1, monitor.match(doc, QueryMatch.SIMPLE_MATCHER).getMatchCount());
+    }
+
+    SimulateUpgradeQuery.HASHCODE_FACTOR = ~StringHelper.GOOD_FAST_HASH_SEED;
+
+    try (Monitor monitor2 = newMonitorWithPersistence(parser)) {
+      assertEquals(1, monitor2.getQueryCount());
+      assertEquals(1, monitor2.match(doc, QueryMatch.SIMPLE_MATCHER).getMatchCount());
+    }
+  }
+
+  private static BooleanQuery incompatibleBooleanQuery(BooleanQuery query) {
+    var booleanBuilder = new BooleanQuery.Builder();
+    for (var clause : query) {
+      booleanBuilder.add(new SimulateUpgradeQuery(clause.query()), BooleanClause.Occur.SHOULD);
+    }
+    return booleanBuilder.build();
+  }
+
+  private static final class SimulateUpgradeQuery extends Query {
+
+    private static volatile int HASHCODE_FACTOR = 1;
+
+    private final Query innerQuery;
+
+    private SimulateUpgradeQuery(Query innerQuery) {
+      this.innerQuery = innerQuery;
+    }
+
+    @Override
+    public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost)
+        throws IOException {
+      return innerQuery.createWeight(searcher, scoreMode, boost);
+    }
+
+    @Override
+    public Query rewrite(IndexSearcher indexSearcher) throws IOException {
+      return innerQuery.rewrite(indexSearcher);
+    }
+
+    @Override
+    public String toString(String field) {
+      return innerQuery.toString(field);
+    }
+
+    @Override
+    public void visit(QueryVisitor visitor) {
+      innerQuery.visit(visitor);
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (!(o instanceof SimulateUpgradeQuery that)) return false;
+      return Objects.equals(innerQuery, that.innerQuery);
+    }
+
+    @Override
+    public int hashCode() {
+      return innerQuery.hashCode() * HASHCODE_FACTOR;
+    }
+  }
 }