Skip to content

Commit 5e30504

Browse files
romseygeekromseygeek
authored
Use inner query for equals/hashCode() in SourceConfirmedTextQuery (#134451) (#134459)
* Use inner query for equals/hashCode() in SourceConfirmedTextQuery (#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 #134432 * Update docs/changelog/134459.yaml * Delete docs/changelog/134459.yaml
1 parent f820acc commit 5e30504

File tree

3 files changed

+47
-37
lines changed

3 files changed

+47
-37
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
@@ -175,14 +175,18 @@ public boolean equals(Object obj) {
175175
return false;
176176
}
177177
SourceConfirmedTextQuery that = (SourceConfirmedTextQuery) obj;
178-
return Objects.equals(in, that.in)
179-
&& Objects.equals(valueFetcherProvider, that.valueFetcherProvider)
180-
&& Objects.equals(indexAnalyzer, that.indexAnalyzer);
178+
// We intentionally do not compare the value fetcher or analyzer, as they
179+
// do not typically implement equals() themselves, and the inner
180+
// Query is sufficient to establish identity.
181+
return Objects.equals(in, that.in);
181182
}
182183

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

188192
@Override

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

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

6060
private static final AtomicInteger sourceFetchCount = new AtomicInteger();
61-
private static final IOFunction<LeafReaderContext, CheckedIntFunction<List<Object>, IOException>> SOURCE_FETCHER_PROVIDER =
62-
context -> docID -> {
61+
62+
private static IOFunction<LeafReaderContext, CheckedIntFunction<List<Object>, IOException>> sourceFetcherProvider() {
63+
return context -> docID -> {
6364
sourceFetchCount.incrementAndGet();
6465
return Collections.<Object>singletonList(context.reader().document(docID).get("body"));
6566
};
67+
}
6668

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

8587
TermQuery query = new TermQuery(new Term("body", "c"));
86-
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
88+
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
8789

8890
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
8991
ScoreDoc[] phraseHits = searcher.search(query, 10).scoreDocs;
@@ -94,7 +96,7 @@ public void testTerm() throws Exception {
9496

9597
// Term query with missing term
9698
query = new TermQuery(new Term("body", "e"));
97-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
99+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
98100
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
99101
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
100102
}
@@ -120,7 +122,7 @@ public void testPhrase() throws Exception {
120122
IndexSearcher searcher = newSearcher(reader);
121123

122124
PhraseQuery query = new PhraseQuery("body", "b", "c");
123-
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
125+
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
124126

125127
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
126128
ScoreDoc[] phraseHits = searcher.search(query, 10).scoreDocs;
@@ -131,7 +133,7 @@ public void testPhrase() throws Exception {
131133

132134
// Sloppy phrase query
133135
query = new PhraseQuery(1, "body", "b", "d");
134-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
136+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
135137
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
136138
phraseHits = searcher.search(query, 10).scoreDocs;
137139
assertEquals(2, phraseHits.length);
@@ -141,13 +143,13 @@ public void testPhrase() throws Exception {
141143

142144
// Phrase query with no matches
143145
query = new PhraseQuery("body", "d", "c");
144-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
146+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
145147
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
146148
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
147149

148150
// Phrase query with one missing term
149151
query = new PhraseQuery("body", "b", "e");
150-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
152+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
151153
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
152154
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
153155
}
@@ -176,7 +178,7 @@ public void testMultiPhrase() throws Exception {
176178
.add(new Term[] { new Term("body", "c") }, 1)
177179
.build();
178180

179-
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
181+
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
180182

181183
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
182184

@@ -191,7 +193,7 @@ public void testMultiPhrase() throws Exception {
191193
.add(new Term[] { new Term("body", "d") }, 1)
192194
.setSlop(1)
193195
.build();
194-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
196+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
195197
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
196198
phraseHits = searcher.search(query, 10).scoreDocs;
197199
assertEquals(2, phraseHits.length);
@@ -203,15 +205,15 @@ public void testMultiPhrase() throws Exception {
203205
query = new MultiPhraseQuery.Builder().add(new Term[] { new Term("body", "d"), new Term("body", "c") }, 0)
204206
.add(new Term[] { new Term("body", "a") }, 1)
205207
.build();
206-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
208+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
207209
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
208210
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
209211

210212
// Multi phrase query with one missing term
211213
query = new MultiPhraseQuery.Builder().add(new Term[] { new Term("body", "d"), new Term("body", "c") }, 0)
212214
.add(new Term[] { new Term("body", "e") }, 1)
213215
.build();
214-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
216+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
215217
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
216218
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
217219
}
@@ -237,7 +239,7 @@ public void testMultiPhrasePrefix() throws Exception {
237239
IndexSearcher searcher = newSearcher(reader);
238240

239241
MultiPhrasePrefixQuery query = new MultiPhrasePrefixQuery("body");
240-
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
242+
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
241243
ScoreDoc[] phrasePrefixHits = searcher.search(query, 10).scoreDocs;
242244
ScoreDoc[] sourceConfirmedHits = searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs;
243245
CheckHits.checkEqual(query, phrasePrefixHits, sourceConfirmedHits);
@@ -246,7 +248,7 @@ public void testMultiPhrasePrefix() throws Exception {
246248

247249
query = new MultiPhrasePrefixQuery("body");
248250
query.add(new Term("body", "c"));
249-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
251+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
250252
phrasePrefixHits = searcher.search(query, 10).scoreDocs;
251253
sourceConfirmedHits = searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs;
252254
CheckHits.checkEqual(query, phrasePrefixHits, sourceConfirmedHits);
@@ -256,7 +258,7 @@ public void testMultiPhrasePrefix() throws Exception {
256258
query = new MultiPhrasePrefixQuery("body");
257259
query.add(new Term("body", "b"));
258260
query.add(new Term("body", "c"));
259-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
261+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
260262
phrasePrefixHits = searcher.search(query, 10).scoreDocs;
261263
sourceConfirmedHits = searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs;
262264
CheckHits.checkEqual(query, phrasePrefixHits, sourceConfirmedHits);
@@ -268,7 +270,7 @@ public void testMultiPhrasePrefix() throws Exception {
268270
query.add(new Term("body", "a"));
269271
query.add(new Term("body", "c"));
270272
query.setSlop(2);
271-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
273+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
272274
phrasePrefixHits = searcher.search(query, 10).scoreDocs;
273275
sourceConfirmedHits = searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs;
274276
CheckHits.checkEqual(query, phrasePrefixHits, sourceConfirmedHits);
@@ -279,15 +281,15 @@ public void testMultiPhrasePrefix() throws Exception {
279281
query = new MultiPhrasePrefixQuery("body");
280282
query.add(new Term("body", "d"));
281283
query.add(new Term("body", "b"));
282-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
284+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
283285
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
284286
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
285287

286288
// Multi phrase query with one missing term
287289
query = new MultiPhrasePrefixQuery("body");
288290
query.add(new Term("body", "d"));
289291
query.add(new Term("body", "f"));
290-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
292+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
291293
assertEquals(0, searcher.count(sourceConfirmedPhraseQuery));
292294
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
293295
}
@@ -317,7 +319,7 @@ public void testSpanNear() throws Exception {
317319
0,
318320
false
319321
);
320-
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
322+
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
321323

322324
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
323325
ScoreDoc[] spanHits = searcher.search(query, 10).scoreDocs;
@@ -332,7 +334,7 @@ public void testSpanNear() throws Exception {
332334
1,
333335
false
334336
);
335-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
337+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
336338
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
337339
spanHits = searcher.search(query, 10).scoreDocs;
338340
assertEquals(2, spanHits.length);
@@ -346,7 +348,7 @@ public void testSpanNear() throws Exception {
346348
0,
347349
false
348350
);
349-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
351+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
350352
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
351353
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
352354

@@ -356,7 +358,7 @@ public void testSpanNear() throws Exception {
356358
0,
357359
false
358360
);
359-
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
361+
sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
360362
assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery));
361363
assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs);
362364
}
@@ -365,30 +367,28 @@ public void testSpanNear() throws Exception {
365367

366368
public void testToString() {
367369
PhraseQuery query = new PhraseQuery("body", "b", "c");
368-
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
370+
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
369371
assertEquals(query.toString(), sourceConfirmedPhraseQuery.toString());
370372
}
371373

372374
public void testEqualsHashCode() {
373375
PhraseQuery query1 = new PhraseQuery("body", "b", "c");
374-
Query sourceConfirmedPhraseQuery1 = new SourceConfirmedTextQuery(query1, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
376+
Query sourceConfirmedPhraseQuery1 = new SourceConfirmedTextQuery(query1, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
375377

376378
assertEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery1);
377379
assertEquals(sourceConfirmedPhraseQuery1.hashCode(), sourceConfirmedPhraseQuery1.hashCode());
378380

379381
PhraseQuery query2 = new PhraseQuery("body", "b", "c");
380-
Query sourceConfirmedPhraseQuery2 = new SourceConfirmedTextQuery(query2, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
382+
Query sourceConfirmedPhraseQuery2 = new SourceConfirmedTextQuery(query2, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
381383
assertEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery2);
382384

383385
PhraseQuery query3 = new PhraseQuery("body", "b", "d");
384-
Query sourceConfirmedPhraseQuery3 = new SourceConfirmedTextQuery(query3, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
386+
Query sourceConfirmedPhraseQuery3 = new SourceConfirmedTextQuery(query3, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
385387
assertNotEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery3);
386388

387-
Query sourceConfirmedPhraseQuery4 = new SourceConfirmedTextQuery(query1, context -> null, Lucene.STANDARD_ANALYZER);
388-
assertNotEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery4);
389-
390-
Query sourceConfirmedPhraseQuery5 = new SourceConfirmedTextQuery(query1, SOURCE_FETCHER_PROVIDER, Lucene.KEYWORD_ANALYZER);
391-
assertNotEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery5);
389+
PhraseQuery query4 = new PhraseQuery("body", "b", "c");
390+
Query sourceConfirmedPhraseQuery6 = new SourceConfirmedTextQuery(query4, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
391+
assertEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery6);
392392
}
393393

394394
public void testApproximation() {
@@ -440,7 +440,7 @@ public void testEmptyIndex() throws Exception {
440440
try (IndexReader reader = DirectoryReader.open(w)) {
441441
IndexSearcher searcher = newSearcher(reader);
442442
PhraseQuery query = new PhraseQuery("body", "a", "b");
443-
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
443+
Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
444444
assertEquals(0, searcher.count(sourceConfirmedPhraseQuery));
445445
}
446446
}
@@ -468,7 +468,7 @@ private static void checkMatches(Query query, String inputDoc, int[] expectedMat
468468
doc.add(new KeywordField("sort", "2", Store.NO));
469469
w.addDocument(doc);
470470

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

473473
try (IndexReader ir = DirectoryReader.open(w)) {
474474
{

0 commit comments

Comments
 (0)