SOLR-18550: Fix for Wrong numFound and numFoundExact for Combined Query results

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

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<ShardDoc> 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,22 +572,29 @@ 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.
    */
   protected Map<Object, ShardDoc> computeResultIdsWithCombiner(
       ResponseBuilder rb,
       Map<String, List<ShardDoc>> 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<ShardDoc> 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);

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",

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(

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