dsmiley commented on code in PR #3849:
URL: https://github.com/apache/solr/pull/3849#discussion_r2508675809


##########
solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java:
##########
@@ -832,20 +832,24 @@ protected void createMainQuery(ResponseBuilder rb) {
     // perhaps we shouldn't attempt to parse the query at this level?
     // Alternate Idea: instead of specifying all these things at the upper 
level,
     // we could just specify that this is a shard request.
+    int shardRows;
     if (rb.shards_rows > -1) {
       // if the client set shards.rows set this explicity
-      sreq.params.set(CommonParams.ROWS, rb.shards_rows);
+      shardRows = rb.shards_rows;
     } else {
-      // what if rows<0 as it is allowed for grouped request??
-      sreq.params.set(
-          CommonParams.ROWS, rb.getSortSpec().getOffset() + 
rb.getSortSpec().getCount());
+      shardRows = rb.getSortSpec().getCount();
+      if (shardRows > 0) {
+        // If rows = -1 (grouped requests) or rows = 0, then there is no need 
to add the offset.

Review Comment:
   the comment is nice but its placement here within this if block is 
confusing.  It applies to the *opposite*.  Please place it before.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to