[solr] branch main updated: SOLR-16176: Never apply facet overrequest in the case where limit:-1 (#830)

[ma...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

magibney pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/main by this push:
     new 81cfb79007d SOLR-16176: Never apply facet overrequest in the case where limit:-1 (#830)
81cfb79007d is described below

commit 81cfb79007d9e179359e206da9dc395b959c5fd1
Author: Michael Gibney <mi...@michaelgibney.net>
AuthorDate: Tue May 3 15:48:00 2022 -0400

    SOLR-16176: Never apply facet overrequest in the case where limit:-1 (#830)
---
 .../FacetFieldProcessorByEnumTermsStream.java      | 24 +++++++++++++---------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessorByEnumTermsStream.java b/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessorByEnumTermsStream.java
index c36449d79a2..fe7421ffe2a 100644
--- a/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessorByEnumTermsStream.java
+++ b/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessorByEnumTermsStream.java
@@ -153,16 +153,20 @@ class FacetFieldProcessorByEnumTermsStream extends FacetFieldProcessor implement
     bucketsToSkip = freq.offset;
 
     effectiveLimit = freq.limit;
-    if (freq.overrequest < -1) {
-      // other negative values are not supported
-      throw new IllegalArgumentException("Illegal `overrequest` specified: " + freq.overrequest);
-    } else if (freq.overrequest > 0 && (fcontext.isShard() || null != resort)) {
-      // NOTE: "index sort" _never_ applies a default overrequest. In both the shard case and the
-      // resort case, the default overrequest is `0`. However, if `overrequest` is explicitly
-      // specified, we respect it except for non-distrib, no-resort request. Overrequest is relevant
-      // for the `resort` case; but it can also be relevant in some edge cases of the "shard" case,
-      // where it can affect the behavior of `isBucketComplete()` (see SOLR-14595).
-      effectiveLimit += freq.overrequest;
+    if (freq.limit >= 0) {
+      // TODO: consider only applying overrequest for `freq.limit > 0` (as opposed to `>=`)?
+      //  corresponding change should be made in overrequest logic for `FacetFieldProcessor`.
+      if (freq.overrequest < -1) {
+        // other negative values are not supported
+        throw new IllegalArgumentException("Illegal `overrequest` specified: " + freq.overrequest);
+      } else if (freq.overrequest > 0 && (fcontext.isShard() || null != resort)) {
+        // NOTE: "index sort" _never_ applies a default overrequest. In both the shard case and the
+        // resort case, the default overrequest is `0`. However, if `overrequest` is explicitly
+        // specified, we respect it except for non-distrib, no-resort request. Overrequest is
+        // relevant for the `resort` case; but it can also be relevant in some edge cases of the
+        // "shard" case, where it can affect the behavior of `isBucketComplete()` (see SOLR-14595).
+        effectiveLimit += freq.overrequest;
+      }
     }
 
     createAccs(-1, 1);