romseygeek commented on code in PR #14885:
URL: https://github.com/apache/lucene/pull/14885#discussion_r2280395257
##########
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() {
Review Comment:
I think we can change `getDisjuncts()` to return things in order rather than
adding an additional method here. We don't make any guarantees about the
ordering at present (and indeed we changed it before in
https://issues.apache.org/jira/browse/LUCENE-9940).
##########
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() {
Review Comment:
Maybe also add a test that asserts that the output of `getDisjuncts()` is
ordered correctly so we don't inadvertently break this again (as I did in the
above mentioned JIRA issue!).
##########
lucene/monitor/src/test/org/apache/lucene/monitor/TestMonitorPersistence.java:
##########
@@ -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;
Review Comment:
This is a nice test!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
