Skip to content

Commit 41fea9d

Browse files
romseygeekromseygeek
authored
Use inner query for equals/hashCode() in SourceConfirmedTextQuery (elastic#134451)
We were using a valueFetcher lambda and an Analyzer as part of the equals and hashCode implementations which could easily compare as unequal for logically equal queries. We can safely just use the inner query for comparisons and hashcodes as the results of the outer query are identical to running the inner query against a shard with indexed positions. Fixes elastic#134432
1 parent 2769584 commit 41fea9d

File tree

3 files changed

+49
-39
lines changed

3 files changed

+49
-39
lines changed

‎docs/changelog/134451.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 134451
2+
summary: Use inner query for equals/hashCode() in `SourceConfirmedTextQuery`
3+
area: "Search"
4+
type: bug
5+
issues:
6+
- 134432

‎modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQuery.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,14 +176,18 @@ public boolean equals(Object obj) {
176176
return false;
177177
}
178178
SourceConfirmedTextQuery that = (SourceConfirmedTextQuery) obj;
179-
return Objects.equals(in, that.in)
180-
&& Objects.equals(valueFetcherProvider, that.valueFetcherProvider)
181-
&& Objects.equals(indexAnalyzer, that.indexAnalyzer);
179+
// We intentionally do not compare the value fetcher or analyzer, as they
180+
// do not typically implement equals() themselves, and the inner
181+
// Query is sufficient to establish identity.
182+
return Objects.equals(in, that.in);
182183
}
183184

184185
@Override
185186
public int hashCode() {
186-
return 31 * Objects.hash(in, valueFetcherProvider, indexAnalyzer) + classHash();
187+
// We intentionally do not hash the value fetcher or analyzer, as they
188+
// do not typically implement hashCode() themselves, and the inner
189+
// Query is sufficient to establish identity.
190+
return 31 * Objects.hash(in) + classHash();
187191
}
188192

189193
@Override

‎modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQueryTests.java‎

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,13 @@
5959
public class SourceConfirmedTextQueryTests extends ESTestCase {
6060

6161
private static final AtomicInteger sourceFetchCount = new AtomicInteger();
62-
private static final IOFunction<LeafReaderContext, CheckedIntFunction<List<Object>, IOException>> SOURCE_FETCHER_PROVIDER =
63-
context -> docID -> {
62+
63+
private static IOFunction<LeafReaderContext, CheckedIntFunction<List<Object>, IOException>> sourceFetcherProvider() {
64+
return context -> docID -> {
6465
sourceFetchCount.incrementAndGet();
65-
return Collections.<Object>singletonList(context.reader().storedFields().document(docID).get("body"));
66+
return Collections.singletonList(context.reader().storedFields().document(docID).get("body"));
6667
};
68+
}
6769

6870
public void testTerm() throws Exception {
6971
try (Directory dir = newDirectory(); IndexWriter w = new IndexWriter(dir, newIndexWriterConfig(Lucene.STANDARD_ANALYZER))) {
@@ -84,7 +86,7 @@ public void testTerm() throws Exception {
8486
IndexSearcher searcher = newSearcher(reader);
8587

8688
TermQuery query = new TermQuery(new Term("body", "c"));
87-
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
89+
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
8890

8991
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
9092
ScoreDoc[] phraseHits = searcher.search(query, 10).scoreDocs;
@@ -95,7 +97,7 @@ public void testTerm() throws Exception {
9597

9698
// Term query with missing term
9799
query = new TermQuery(new Term("body", "e"));
98-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
100+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
99101
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
100102
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
101103
}
@@ -112,7 +114,7 @@ public void testMissingPhrase() throws Exception {
112114
try (IndexReader reader = DirectoryReader.open(w)) {
113115
IndexSearcher searcher = newSearcher(reader);
114116
PhraseQuery query = new PhraseQuery("missing_field", "b", "c");
115-
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
117+
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
116118
Explanation explanation = searcher.explain(sourceConfirmedPhraseQuery, 0);
117119
assertFalse(explanation.isMatch());
118120

@@ -141,7 +143,7 @@ public void testPhrase() throws Exception {
141143
IndexSearcher searcher = newSearcher(reader);
142144

143145
PhraseQuery query = new PhraseQuery("body", "b", "c");
144-
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
146+
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
145147

146148
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
147149
ScoreDoc[] phraseHits = searcher.search(query, 10).scoreDocs;
@@ -152,7 +154,7 @@ public void testPhrase() throws Exception {
152154

153155
// Sloppy phrase query
154156
query = new PhraseQuery(1, "body", "b", "d");
155-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
157+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
156158
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
157159
phraseHits = searcher.search(query, 10).scoreDocs;
158160
assertEquals(2, phraseHits.length);
@@ -162,13 +164,13 @@ public void testPhrase() throws Exception {
162164

163165
// Phrase query with no matches
164166
query = new PhraseQuery("body", "d", "c");
165-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
167+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
166168
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
167169
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
168170

169171
// Phrase query with one missing term
170172
query = new PhraseQuery("body", "b", "e");
171-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
173+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
172174
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
173175
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
174176
}
@@ -197,7 +199,7 @@ public void testMultiPhrase() throws Exception {
197199
.add(new Term[] { new Term("body", "c") }, 1)
198200
.build();
199201

200-
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
202+
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
201203

202204
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
203205

@@ -212,7 +214,7 @@ public void testMultiPhrase() throws Exception {
212214
.add(new Term[] { new Term("body", "d") }, 1)
213215
.setSlop(1)
214216
.build();
215-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
217+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
216218
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
217219
phraseHits = searcher.search(query, 10).scoreDocs;
218220
assertEquals(2, phraseHits.length);
@@ -224,15 +226,15 @@ public void testMultiPhrase() throws Exception {
224226
query = new MultiPhraseQuery.Builder().add(new Term[] { new Term("body", "d"), new Term("body", "c") }, 0)
225227
.add(new Term[] { new Term("body", "a") }, 1)
226228
.build();
227-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
229+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
228230
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
229231
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
230232

231233
// Multi phrase query with one missing term
232234
query = new MultiPhraseQuery.Builder().add(new Term[] { new Term("body", "d"), new Term("body", "c") }, 0)
233235
.add(new Term[] { new Term("body", "e") }, 1)
234236
.build();
235-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
237+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
236238
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
237239
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
238240
}
@@ -258,7 +260,7 @@ public void testMultiPhrasePrefix() throws Exception {
258260
IndexSearcher searcher = newSearcher(reader);
259261

260262
MultiPhrasePrefixQuery query = new MultiPhrasePrefixQuery("body");
261-
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
263+
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
262264
ScoreDoc[] phrasePrefixHits = searcher.search(query, 10).scoreDocs;
263265
ScoreDoc[] sourceConfirmedHits = searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs;
264266
CheckHits.checkEqual(query, phrasePrefixHits, sourceConfirmedHits);
@@ -267,7 +269,7 @@ public void testMultiPhrasePrefix() throws Exception {
267269

268270
query = new MultiPhrasePrefixQuery("body");
269271
query.add(new Term("body", "c"));
270-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
272+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
271273
phrasePrefixHits = searcher.search(query, 10).scoreDocs;
272274
sourceConfirmedHits = searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs;
273275
CheckHits.checkEqual(query, phrasePrefixHits, sourceConfirmedHits);
@@ -277,7 +279,7 @@ public void testMultiPhrasePrefix() throws Exception {
277279
query = new MultiPhrasePrefixQuery("body");
278280
query.add(new Term("body", "b"));
279281
query.add(new Term("body", "c"));
280-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
282+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
281283
phrasePrefixHits = searcher.search(query, 10).scoreDocs;
282284
sourceConfirmedHits = searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs;
283285
CheckHits.checkEqual(query, phrasePrefixHits, sourceConfirmedHits);
@@ -289,7 +291,7 @@ public void testMultiPhrasePrefix() throws Exception {
289291
query.add(new Term("body", "a"));
290292
query.add(new Term("body", "c"));
291293
query.setSlop(2);
292-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
294+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
293295
phrasePrefixHits = searcher.search(query, 10).scoreDocs;
294296
sourceConfirmedHits = searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs;
295297
CheckHits.checkEqual(query, phrasePrefixHits, sourceConfirmedHits);
@@ -300,15 +302,15 @@ public void testMultiPhrasePrefix() throws Exception {
300302
query = new MultiPhrasePrefixQuery("body");
301303
query.add(new Term("body", "d"));
302304
query.add(new Term("body", "b"));
303-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
305+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
304306
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
305307
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
306308

307309
// Multi phrase query with one missing term
308310
query = new MultiPhrasePrefixQuery("body");
309311
query.add(new Term("body", "d"));
310312
query.add(new Term("body", "f"));
311-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
313+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
312314
assertEquals(0, searcher.count(sourceConfirmedPhraseQuery));
313315
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
314316
}
@@ -338,7 +340,7 @@ public void testSpanNear() throws Exception {
338340
0,
339341
false
340342
);
341-
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
343+
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
342344

343345
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
344346
ScoreDoc[] spanHits = searcher.search(query, 10).scoreDocs;
@@ -353,7 +355,7 @@ public void testSpanNear() throws Exception {
353355
1,
354356
false
355357
);
356-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
358+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
357359
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
358360
spanHits = searcher.search(query, 10).scoreDocs;
359361
assertEquals(2, spanHits.length);
@@ -367,7 +369,7 @@ public void testSpanNear() throws Exception {
367369
0,
368370
false
369371
);
370-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
372+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
371373
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
372374
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
373375

@@ -377,7 +379,7 @@ public void testSpanNear() throws Exception {
377379
0,
378380
false
379381
);
380-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
382+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
381383
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
382384
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
383385
}
@@ -386,30 +388,28 @@ public void testSpanNear() throws Exception {
386388

387389
public void testToString() {
388390
PhraseQuery query = new PhraseQuery("body", "b", "c");
389-
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
391+
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
390392
assertEquals(query.toString(), sourceConfirmedPhraseQuery.toString());
391393
}
392394

393395
public void testEqualsHashCode() {
394396
PhraseQuery query1 = new PhraseQuery("body", "b", "c");
395-
Query sourceConfirmedPhraseQuery1 = new SourceConfirmedTextQuery(query1, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
397+
Query sourceConfirmedPhraseQuery1 = new SourceConfirmedTextQuery(query1, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
396398

397399
assertEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery1);
398400
assertEquals(sourceConfirmedPhraseQuery1.hashCode(), sourceConfirmedPhraseQuery1.hashCode());
399401

400402
PhraseQuery query2 = new PhraseQuery("body", "b", "c");
401-
Query sourceConfirmedPhraseQuery2 = new SourceConfirmedTextQuery(query2, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
403+
Query sourceConfirmedPhraseQuery2 = new SourceConfirmedTextQuery(query2, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
402404
assertEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery2);
403405

404406
PhraseQuery query3 = new PhraseQuery("body", "b", "d");
405-
Query sourceConfirmedPhraseQuery3 = new SourceConfirmedTextQuery(query3, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
407+
Query sourceConfirmedPhraseQuery3 = new SourceConfirmedTextQuery(query3, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
406408
assertNotEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery3);
407409

408-
Query sourceConfirmedPhraseQuery4 = new SourceConfirmedTextQuery(query1, context -> null, Lucene.STANDARD_ANALYZER);
409-
assertNotEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery4);
410-
411-
Query sourceConfirmedPhraseQuery5 = new SourceConfirmedTextQuery(query1, SOURCE_FETCHER_PROVIDER, Lucene.KEYWORD_ANALYZER);
412-
assertNotEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery5);
410+
PhraseQuery query4 = new PhraseQuery("body", "b", "c");
411+
Query sourceConfirmedPhraseQuery6 = new SourceConfirmedTextQuery(query4, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
412+
assertEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery6);
413413
}
414414

415415
public void testApproximation() {
@@ -461,7 +461,7 @@ public void testEmptyIndex() throws Exception {
461461
try (IndexReader reader = DirectoryReader.open(w)) {
462462
IndexSearcher searcher = newSearcher(reader);
463463
PhraseQuery query = new PhraseQuery("body", "a", "b");
464-
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
464+
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
465465
assertEquals(0, searcher.count(sourceConfirmedPhraseQuery));
466466
}
467467
}
@@ -489,7 +489,7 @@ private static void checkMatches(Query query, String inputDoc, int[] expectedMat
489489
doc.add(new KeywordField("sort", "2", Store.NO));
490490
w.addDocument(doc);
491491

492-
Query sourceConfirmedQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
492+
Query sourceConfirmedQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
493493

494494
try (IndexReader ir = DirectoryReader.open(w)) {
495495
{

0 commit comments

Comments
 (0)