From f03dd9c6b01dcb676e36110d3f87435c0f73fa96 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 10 Sep 2025 15:50:23 +0100 Subject: [PATCH 1/3] 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 --- docs/changelog/134451.yaml | 6 ++ .../extras/SourceConfirmedTextQuery.java | 12 ++-- .../extras/SourceConfirmedTextQueryTests.java | 66 +++++++++---------- 3 files changed, 47 insertions(+), 37 deletions(-) create mode 100644 docs/changelog/134451.yaml diff --git a/docs/changelog/134451.yaml b/docs/changelog/134451.yaml new file mode 100644 index 0000000000000..94fec239548c9 --- /dev/null +++ b/docs/changelog/134451.yaml @@ -0,0 +1,6 @@ +pr: 134451 +summary: Use inner query for equals/hashCode() in `SourceConfirmedTextQuery` +area: "Search" +type: bug +issues: + - 134432 diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQuery.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQuery.java index 34bb30d581aaa..60f3aa09aa7fd 100644 --- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQuery.java +++ b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQuery.java @@ -175,14 +175,18 @@ public boolean equals(Object obj) { return false; } SourceConfirmedTextQuery that = (SourceConfirmedTextQuery) obj; - return Objects.equals(in, that.in) - && Objects.equals(valueFetcherProvider, that.valueFetcherProvider) - && Objects.equals(indexAnalyzer, that.indexAnalyzer); + // We intentionally do not compare the value fetcher or analyzer, as they + // do not typically implement equals() themselves, and the inner + // Query is sufficient to establish identity. + return Objects.equals(in, that.in); } @Override public int hashCode() { - return 31 * Objects.hash(in, valueFetcherProvider, indexAnalyzer) + classHash(); + // We intentionally do not hash the value fetcher or analyzer, as they + // do not typically implement hashCode() themselves, and the inner + // Query is sufficient to establish identity. + return 31 * Objects.hash(in) + classHash(); } @Override diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQueryTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQueryTests.java index 84139409e8bc6..ee0defe7bb040 100644 --- a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQueryTests.java +++ b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQueryTests.java @@ -58,11 +58,13 @@ public class SourceConfirmedTextQueryTests extends ESTestCase { private static final AtomicInteger sourceFetchCount = new AtomicInteger(); - private static final IOFunction, IOException>> SOURCE_FETCHER_PROVIDER = - context -> docID -> { + + private static IOFunction, IOException>> sourceFetcherProvider() { + return context -> docID -> { sourceFetchCount.incrementAndGet(); return Collections.singletonList(context.reader().document(docID).get("body")); }; + } public void testTerm() throws Exception { try (Directory dir = newDirectory(); IndexWriter w = new IndexWriter(dir, newIndexWriterConfig(Lucene.STANDARD_ANALYZER))) { @@ -83,7 +85,7 @@ public void testTerm() throws Exception { IndexSearcher searcher = newSearcher(reader); TermQuery query = new TermQuery(new Term("body", "c")); - Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery)); ScoreDoc[] phraseHits = searcher.search(query, 10).scoreDocs; @@ -94,7 +96,7 @@ public void testTerm() throws Exception { // Term query with missing term query = new TermQuery(new Term("body", "e")); - sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery)); assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs); } @@ -120,7 +122,7 @@ public void testPhrase() throws Exception { IndexSearcher searcher = newSearcher(reader); PhraseQuery query = new PhraseQuery("body", "b", "c"); - Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery)); ScoreDoc[] phraseHits = searcher.search(query, 10).scoreDocs; @@ -131,7 +133,7 @@ public void testPhrase() throws Exception { // Sloppy phrase query query = new PhraseQuery(1, "body", "b", "d"); - sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery)); phraseHits = searcher.search(query, 10).scoreDocs; assertEquals(2, phraseHits.length); @@ -141,13 +143,13 @@ public void testPhrase() throws Exception { // Phrase query with no matches query = new PhraseQuery("body", "d", "c"); - sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery)); assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs); // Phrase query with one missing term query = new PhraseQuery("body", "b", "e"); - sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery)); assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs); } @@ -176,7 +178,7 @@ public void testMultiPhrase() throws Exception { .add(new Term[] { new Term("body", "c") }, 1) .build(); - Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery)); @@ -191,7 +193,7 @@ public void testMultiPhrase() throws Exception { .add(new Term[] { new Term("body", "d") }, 1) .setSlop(1) .build(); - sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery)); phraseHits = searcher.search(query, 10).scoreDocs; assertEquals(2, phraseHits.length); @@ -203,7 +205,7 @@ public void testMultiPhrase() throws Exception { query = new MultiPhraseQuery.Builder().add(new Term[] { new Term("body", "d"), new Term("body", "c") }, 0) .add(new Term[] { new Term("body", "a") }, 1) .build(); - sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery)); assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs); @@ -211,7 +213,7 @@ public void testMultiPhrase() throws Exception { query = new MultiPhraseQuery.Builder().add(new Term[] { new Term("body", "d"), new Term("body", "c") }, 0) .add(new Term[] { new Term("body", "e") }, 1) .build(); - sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery)); assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs); } @@ -237,7 +239,7 @@ public void testMultiPhrasePrefix() throws Exception { IndexSearcher searcher = newSearcher(reader); MultiPhrasePrefixQuery query = new MultiPhrasePrefixQuery("body"); - Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); ScoreDoc[] phrasePrefixHits = searcher.search(query, 10).scoreDocs; ScoreDoc[] sourceConfirmedHits = searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs; CheckHits.checkEqual(query, phrasePrefixHits, sourceConfirmedHits); @@ -246,7 +248,7 @@ public void testMultiPhrasePrefix() throws Exception { query = new MultiPhrasePrefixQuery("body"); query.add(new Term("body", "c")); - sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); phrasePrefixHits = searcher.search(query, 10).scoreDocs; sourceConfirmedHits = searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs; CheckHits.checkEqual(query, phrasePrefixHits, sourceConfirmedHits); @@ -256,7 +258,7 @@ public void testMultiPhrasePrefix() throws Exception { query = new MultiPhrasePrefixQuery("body"); query.add(new Term("body", "b")); query.add(new Term("body", "c")); - sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); phrasePrefixHits = searcher.search(query, 10).scoreDocs; sourceConfirmedHits = searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs; CheckHits.checkEqual(query, phrasePrefixHits, sourceConfirmedHits); @@ -268,7 +270,7 @@ public void testMultiPhrasePrefix() throws Exception { query.add(new Term("body", "a")); query.add(new Term("body", "c")); query.setSlop(2); - sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); phrasePrefixHits = searcher.search(query, 10).scoreDocs; sourceConfirmedHits = searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs; CheckHits.checkEqual(query, phrasePrefixHits, sourceConfirmedHits); @@ -279,7 +281,7 @@ public void testMultiPhrasePrefix() throws Exception { query = new MultiPhrasePrefixQuery("body"); query.add(new Term("body", "d")); query.add(new Term("body", "b")); - sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery)); assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs); @@ -287,7 +289,7 @@ public void testMultiPhrasePrefix() throws Exception { query = new MultiPhrasePrefixQuery("body"); query.add(new Term("body", "d")); query.add(new Term("body", "f")); - sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(0, searcher.count(sourceConfirmedPhraseQuery)); assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs); } @@ -317,7 +319,7 @@ public void testSpanNear() throws Exception { 0, false ); - Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery)); ScoreDoc[] spanHits = searcher.search(query, 10).scoreDocs; @@ -332,7 +334,7 @@ public void testSpanNear() throws Exception { 1, false ); - sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery)); spanHits = searcher.search(query, 10).scoreDocs; assertEquals(2, spanHits.length); @@ -346,7 +348,7 @@ public void testSpanNear() throws Exception { 0, false ); - sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery)); assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs); @@ -356,7 +358,7 @@ public void testSpanNear() throws Exception { 0, false ); - sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(searcher.count(query), searcher.count(sourceConfirmedPhraseQuery)); assertArrayEquals(new ScoreDoc[0], searcher.search(sourceConfirmedPhraseQuery, 10).scoreDocs); } @@ -365,30 +367,28 @@ public void testSpanNear() throws Exception { public void testToString() { PhraseQuery query = new PhraseQuery("body", "b", "c"); - Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(query.toString(), sourceConfirmedPhraseQuery.toString()); } public void testEqualsHashCode() { PhraseQuery query1 = new PhraseQuery("body", "b", "c"); - Query sourceConfirmedPhraseQuery1 = new SourceConfirmedTextQuery(query1, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + Query sourceConfirmedPhraseQuery1 = new SourceConfirmedTextQuery(query1, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery1); assertEquals(sourceConfirmedPhraseQuery1.hashCode(), sourceConfirmedPhraseQuery1.hashCode()); PhraseQuery query2 = new PhraseQuery("body", "b", "c"); - Query sourceConfirmedPhraseQuery2 = new SourceConfirmedTextQuery(query2, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + Query sourceConfirmedPhraseQuery2 = new SourceConfirmedTextQuery(query2, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery2); PhraseQuery query3 = new PhraseQuery("body", "b", "d"); - Query sourceConfirmedPhraseQuery3 = new SourceConfirmedTextQuery(query3, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + Query sourceConfirmedPhraseQuery3 = new SourceConfirmedTextQuery(query3, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertNotEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery3); - Query sourceConfirmedPhraseQuery4 = new SourceConfirmedTextQuery(query1, context -> null, Lucene.STANDARD_ANALYZER); - assertNotEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery4); - - Query sourceConfirmedPhraseQuery5 = new SourceConfirmedTextQuery(query1, SOURCE_FETCHER_PROVIDER, Lucene.KEYWORD_ANALYZER); - assertNotEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery5); + PhraseQuery query4 = new PhraseQuery("body", "b", "c"); + Query sourceConfirmedPhraseQuery6 = new SourceConfirmedTextQuery(query4, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); + assertEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery6); } public void testApproximation() { @@ -440,7 +440,7 @@ public void testEmptyIndex() throws Exception { try (IndexReader reader = DirectoryReader.open(w)) { IndexSearcher searcher = newSearcher(reader); PhraseQuery query = new PhraseQuery("body", "a", "b"); - Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); assertEquals(0, searcher.count(sourceConfirmedPhraseQuery)); } } @@ -468,7 +468,7 @@ private static void checkMatches(Query query, String inputDoc, int[] expectedMat doc.add(new KeywordField("sort", "2", Store.NO)); w.addDocument(doc); - Query sourceConfirmedQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER); + Query sourceConfirmedQuery = new SourceConfirmedTextQuery(query, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER); try (IndexReader ir = DirectoryReader.open(w)) { { From 728498db4f9f52c9d8a002386ee4726a3b5e5633 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 10 Sep 2025 16:22:34 +0100 Subject: [PATCH 2/3] Update docs/changelog/134459.yaml --- docs/changelog/134459.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/134459.yaml diff --git a/docs/changelog/134459.yaml b/docs/changelog/134459.yaml new file mode 100644 index 0000000000000..b9a1a6ab18eec --- /dev/null +++ b/docs/changelog/134459.yaml @@ -0,0 +1,6 @@ +pr: 134459 +summary: Use inner query for equals/hashCode() in `SourceConfirmedTextQuery` +area: Search +type: bug +issues: + - 134432 From ddb7d0c60f8a3000fe8fe87273be0181aec97063 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 10 Sep 2025 16:29:51 +0100 Subject: [PATCH 3/3] Delete docs/changelog/134459.yaml --- docs/changelog/134459.yaml | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 docs/changelog/134459.yaml diff --git a/docs/changelog/134459.yaml b/docs/changelog/134459.yaml deleted file mode 100644 index b9a1a6ab18eec..0000000000000 --- a/docs/changelog/134459.yaml +++ /dev/null @@ -1,6 +0,0 @@ -pr: 134459 -summary: Use inner query for equals/hashCode() in `SourceConfirmedTextQuery` -area: Search -type: bug -issues: - - 134432