diff --git a/changelog/unreleased/SOLR-18550-combined-query-numfound.yml b/changelog/unreleased/SOLR-18550-combined-query-numfound.yml new file mode 100644 index 000000000000..702ef9458db5 --- /dev/null +++ b/changelog/unreleased/SOLR-18550-combined-query-numfound.yml @@ -0,0 +1,13 @@ +title: > + For combined queries, numFound is the distinct fused document count when numFoundExact + is true, and otherwise a lower bound of at least the largest sub-query's match count, + marked inexact when a sub-query list was truncated by the fusion pool (shards.rows + or start + rows). +type: fixed +authors: + - name: Puneet Ahuja + nick: punAhuja + - name: Justin Michael Coyne +links: + - name: SOLR-18550 + url: https://issues.apache.org/jira/browse/SOLR-18550 \ No newline at end of file diff --git a/solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java b/solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java index 5447b57a79cb..b4b956d77452 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java @@ -311,8 +311,9 @@ protected void mergeIds(ResponseBuilder rb, ShardRequest sreq) { rb.rsp.getValues().add(ShardParams.SHARDS_INFO, shardInfo); } - long numFound = 0; boolean hitCountIsExact = true; + boolean someQueryListTruncated = false; + long maxQueryNumFound = 0; boolean thereArePartialResults = false; Boolean segmentTerminatedEarly = null; boolean maxHitsTerminatedEarly = false; @@ -340,7 +341,9 @@ protected void mergeIds(ResponseBuilder rb, ShardRequest sreq) { && docList.get(queryIndex) instanceof SolrDocumentList solrDocumentList) { docs = Objects.requireNonNull(solrDocumentList); queryNumFound += docs.getNumFound(); - hitCountIsExact = hitCountIsExact && Boolean.FALSE.equals(docs.getNumFoundExact()); + if (hitCountIsExact && Boolean.FALSE.equals(docs.getNumFoundExact())) { + hitCountIsExact = false; + } } failedShardCount += addShardInfo( @@ -430,27 +433,26 @@ protected void mergeIds(ResponseBuilder rb, ShardRequest sreq) { shardDoc.scoreDependentFields = doc.getSubsetOfFields(scoreDependentFields); } shardDoc.sortFieldValues = unmarshalledSortFieldValues; - if (!queuePerQuery.push(shardDoc)) { - numFound--; - } + queuePerQuery.push(shardDoc); } // end for-each-doc-in-response } // end for-each-response List shardDocsPerQuery = new ArrayList<>(queuePerQuery.resultIds(0).values()); shardDocsPerQuery.sort(Comparator.comparingInt(a -> a.positionInResponse)); shardDocMap.put(queriesToCombineKeys[queryIndex], shardDocsPerQuery); - numFound = max(numFound, queryNumFound); + // a sub-query list is complete iff every match reached the fusion pool + someQueryListTruncated |= queryNumFound != shardDocsPerQuery.size(); + maxQueryNumFound = max(maxQueryNumFound, queryNumFound); approximateTotalHits = max(approximateTotalHits, queryApproximateTotalHits); } // for each query to combine - rb.rsp.addToLog("hits", numFound); - SolrDocumentList responseDocs = new SolrDocumentList(); - responseDocs.setNumFound(numFound); - responseDocs.setNumFoundExact(hitCountIsExact); + responseDocs.setNumFoundExact(hitCountIsExact && !someQueryListTruncated); responseDocs.setStart(ss.getOffset()); - rb.resultIds = computeResultIdsWithCombiner(rb, shardDocMap, responseDocs, sortFields); + rb.resultIds = + computeResultIdsWithCombiner(rb, shardDocMap, responseDocs, sortFields, maxQueryNumFound); rb.setResponseDocs(responseDocs); + rb.rsp.addToLog("hits", responseDocs.getNumFound()); populateNextCursorMarkFromMergedShards(rb); @@ -570,8 +572,10 @@ private int addShardInfo( * @param rb The ResponseBuilder containing the request and context, such as sort specifications. * @param shardDocMap A map from shard addresses to the list of documents returned by each shard. * @param responseDocs The final response document list, which will be populated with null - * placeholders and have its max score set. + * placeholders and have its numFound and max score set. * @param sortFields An array of field for sorting to be applied. + * @param maxQueryNumFound The largest sub-query's match count, summed across shards; a floor for + * the reported numFound. * @return A map from document IDs to the corresponding ShardDoc objects for the documents in the * final sorted page of results. */ @@ -579,13 +583,18 @@ protected Map computeResultIdsWithCombiner( ResponseBuilder rb, Map> shardDocMap, SolrDocumentList responseDocs, - SortField[] sortFields) { + SortField[] sortFields, + long maxQueryNumFound) { String algorithm = rb.req.getParams().get(CombinerParams.COMBINER_ALGORITHM, CombinerParams.DEFAULT_COMBINER); QueryAndResponseCombiner combinerStrategy = QueryAndResponseCombiner.getImplementation(algorithm, combiners); List combinedShardDocs = combinerStrategy.combine(shardDocMap, rb.req.getParams()); + // the fused list is deduped by doc id, so its size is the distinct doc count; numFound is + // that count, but never below the largest sub-query's match count + responseDocs.setNumFound(max(maxQueryNumFound, combinedShardDocs.size())); + // adding explanation for the ordered shard docs as debug info if (rb.isDebugResults()) { String[] queryKeys = rb.req.getParams().getParams(CombinerParams.COMBINER_QUERY); diff --git a/solr/core/src/test/org/apache/solr/handler/component/CombinedQueryComponentTest.java b/solr/core/src/test/org/apache/solr/handler/component/CombinedQueryComponentTest.java index 4bf7b18fcf93..61bcccb2829b 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/CombinedQueryComponentTest.java +++ b/solr/core/src/test/org/apache/solr/handler/component/CombinedQueryComponentTest.java @@ -127,16 +127,44 @@ public void testMultipleLexicalQueryWithDebug() throws Exception { params( CommonParams.JSON, "{\"queries\":" - + "{\"lexical1\":{\"lucene\":{\"query\":\"title:title test for doc 1\"}}," - + "\"lexical2\":{\"lucene\":{\"query\":\"text:test text for doc 2\"}}}," + + "{\"lexical1\":{\"lucene\":{\"query\":\"id:(2^=4 OR 3^=2 OR 6^=3 OR 5^=1)\"}}," + + "\"lexical2\":{\"lucene\":{\"query\":\"id:(4^=1 OR 5^=3 OR 7^=4 OR 10^=2)\"}}}," + "\"limit\":5," + "\"fields\":[\"id\",\"score\",\"title\"]," + "\"params\":{\"combiner\":true,\"debug\":[\"results\"],\"combiner.query\":[\"lexical1\",\"lexical2\"]," + "\"rid\": \"test-1\"}}")); - assertEquals(10, rsp.getResults().getNumFound()); + // both queries match 4 docs, overlapping on doc 5, so the fused result has 7 distinct docs + assertEquals(7, rsp.getResults().getNumFound()); assertTrue(rsp.getDebugMap().containsKey("combinerExplanations")); } + /** + * Tests that numFound is the distinct count of fused documents, not the larger sub-query's match + * count. Two queries matching three and two distinct docs, no overlap. + */ + @Test + public void testNumFoundIsFusedCount() throws Exception { + prepareIndexDocs(); + QueryResponse rsp = + query( + "/search", + params( + CommonParams.JSON, + """ + { + "queries": { + "cats": {"lucene": {"query": "id:(1^=3 OR 2^=2 OR 3^=1)"}}, + "dogs": {"lucene": {"query": "id:(4^=2 OR 5^=1)"}} + }, + "limit": 10, + "fields": ["id"], + "params": {"combiner": true, "combiner.query": ["cats", "dogs"]} + }""")); + assertEquals(5, rsp.getResults().size()); + assertEquals(5, rsp.getResults().getNumFound()); + assertEquals(Boolean.TRUE, rsp.getResults().getNumFoundExact()); + } + /** Test no results in combined queries. */ @Test public void testNoResults() throws Exception { @@ -196,6 +224,8 @@ public void testCombinerPlugin() throws Exception { + "\"fields\":[\"id\",\"score\",\"title\"]," + "\"params\":{\"combiner\":true,\"combiner.algorithm\":test,\"combiner.query\"" + ":[\"lexical1\",\"lexical2\"],\"debug\":[\"results\"]}}")); + // the test combiner fuses to an empty list, so numFound falls back to the largest + // sub-query's match count rather than the (empty) fused pool assertEquals(10, rsp.getResults().getNumFound()); assertEquals( "org.apache.lucene.search.Explanation:30 = this is test combiner\n", diff --git a/solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java b/solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java index 3c8044f46198..1c901d648b08 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java +++ b/solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java @@ -162,6 +162,69 @@ public void testCombinedQueries() throws Exception { assertEquals(5, rsp.getResults().size()); assertFieldValues(rsp.getResults(), id, "5", "7", "2", "6", "3"); + // both queries match 6 docs, overlapping on docs 5 and 6, so the fused result has 10 distinct + // docs; numFound is that distinct count, exact since no list was truncated + rsp = + query( + "/search", + params( + CommonParams.JSON, + """ + { + "queries": { + "lexical1": {"lucene": {"query": "id:(1^=6 OR 2^=5 OR 3^=4 OR 4^=3 OR 5^=2 OR 6^=1)"}}, + "lexical2": {"lucene": {"query": "id:(5^=6 OR 6^=5 OR 7^=4 OR 8^=3 OR 9^=2 OR 10^=1)"}} + }, + "limit": 10, + "fields": ["id", "score"], + "params": {"combiner": true, "combiner.query": ["lexical1", "lexical2"]} + }""")); + assertEquals(10, rsp.getResults().getNumFound()); + assertEquals(Boolean.TRUE, rsp.getResults().getNumFoundExact()); + + // same queries, but the fusion pool (start + rows = 2) truncates each 6-match list to its + // top 2; numFound never drops below the largest sub-query's match count (6) and is + // reported inexact because the fused pool holds only a subset of the matches + rsp = + query( + "/search", + params( + CommonParams.JSON, + """ + { + "queries": { + "lexical1": {"lucene": {"query": "id:(1^=6 OR 2^=5 OR 3^=4 OR 4^=3 OR 5^=2 OR 6^=1)"}}, + "lexical2": {"lucene": {"query": "id:(5^=6 OR 6^=5 OR 7^=4 OR 8^=3 OR 9^=2 OR 10^=1)"}} + }, + "limit": 2, + "fields": ["id", "score"], + "params": {"combiner": true, "combiner.query": ["lexical1", "lexical2"]} + }""")); + assertEquals(2, rsp.getResults().size()); + assertEquals(6, rsp.getResults().getNumFound()); + assertEquals(Boolean.FALSE, rsp.getResults().getNumFoundExact()); + + // count-only: rows=0 leaves the fusion pool empty, but numFound still reports the + // largest sub-query's match count rather than the pool size of 0 + rsp = + query( + "/search", + params( + CommonParams.JSON, + """ + { + "queries": { + "lexical1": {"lucene": {"query": "id:(1^=6 OR 2^=5 OR 3^=4 OR 4^=3 OR 5^=2 OR 6^=1)"}}, + "lexical2": {"lucene": {"query": "id:(5^=6 OR 6^=5 OR 7^=4 OR 8^=3 OR 9^=2 OR 10^=1)"}} + }, + "limit": 0, + "fields": ["id", "score"], + "params": {"combiner": true, "combiner.query": ["lexical1", "lexical2"]} + }""")); + assertEquals(0, rsp.getResults().size()); + assertEquals(6, rsp.getResults().getNumFound()); + assertEquals(Boolean.FALSE, rsp.getResults().getNumFoundExact()); + // Multiple queries with sort rsp = query( diff --git a/solr/solr-ref-guide/modules/query-guide/pages/json-combined-query-dsl.adoc b/solr/solr-ref-guide/modules/query-guide/pages/json-combined-query-dsl.adoc index 33f841cd2cc6..2c009c4cb5fc 100644 --- a/solr/solr-ref-guide/modules/query-guide/pages/json-combined-query-dsl.adoc +++ b/solr/solr-ref-guide/modules/query-guide/pages/json-combined-query-dsl.adoc @@ -79,6 +79,9 @@ The query structure is similar to JSON Query DSL except for how multiple queries [NOTE] ==== Consider setting `shards.rows` to at least your deepest expected `start + rows` to enlarge the per-shard candidate pool used for fusion, giving more consistent ordering across pages. + +`numFound` in the response is the number of distinct documents in the fused result when `numFoundExact` is true. +Otherwise it is a lower bound — at least the largest sub-query's match count — because a sub-query matched more documents than were retained for fusion (`shards.rows` or `start + rows`). ==== === Example