[GitHub] [solr] thelabdude commented on a change in pull request #191: SOLR-15489: Implement OFFSET & FETCH for LIMIT queries

[GitBox <gi...@apache.org>]

thelabdude commented on a change in pull request #191:
URL: https://github.com/apache/solr/pull/191#discussion_r658265207



##########
File path: solr/core/src/java/org/apache/solr/handler/sql/SolrTable.java
##########
@@ -275,25 +277,43 @@ private TupleStream handleSelect(String zk,
 
     String fl = getFields(fields);
 
-    if(orders.size() > 0) {
+    if (!orders.isEmpty()) {
       params.add(SORT, getSort(orders));
     } else {
-      if(limit == null) {
+      if (limit == null) {
         params.add(SORT, "_version_ desc");
         fl = fl+",_version_";
       } else {
         params.add(SORT, "score desc");
-        if(fl.indexOf("score") == -1) {
+        if (!fl.contains("score")) {
           fl = fl + ",score";
         }
       }
     }
 
     params.add(CommonParams.FL, fl);
-
+    
+    if (offset != null && limit == null) {
+      throw new IOException("OFFSET without LIMIT not supported by Solr! Specify desired limit using 'FETCH NEXT <LIMIT> ROWS ONLY'");
+    }
+    
     if (limit != null) {
-      params.add(CommonParams.ROWS, limit);
-      return new LimitStream(new CloudSolrStream(zk, collection, params), Integer.parseInt(limit));
+      int limitInt = Integer.parseInt(limit);
+      // if there's an offset, then we need to fetch offset + limit rows from each shard and then sort accordingly
+      LimitStream limitStream;
+      if (offset != null) {
+        int offsetInt = Integer.parseInt(offset);
+        int rows = limitInt + offsetInt;
+        params.add(CommonParams.START, "0"); // tricky ... we need all rows up to limit + offset

Review comment:
       How would you map a cursor mark to SQL? Not sure why this makes you nervous as it's not exposing any more risk than people doing:
   ```
   q=*:*&start=1000000000000&rows=10
   ```
   Solr's query interface allows all kinds of dangerous input if people want to mis-use the parameters.
   
   If you have an idea on how to map cursorMark to SQL I'd appreciate hearing the details.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org