vrajat commented on code in PR #14110:
URL: https://github.com/apache/pinot/pull/14110#discussion_r1808653546


##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java:
##########
@@ -437,6 +456,22 @@ private BrokerResponse executeSqlQuery(ObjectNode 
sqlRequestJson, HttpRequesterI
     if (forceUseMultiStage) {
       
sqlNodeAndOptions.setExtraOptions(ImmutableMap.of(Request.QueryOptionKey.USE_MULTISTAGE_ENGINE,
 "true"));
     }
+    if (getCursor != null && getCursor) {
+      if (numRows == null) {
+        numRows = 
_brokerConf.getProperty(CommonConstants.CursorConfigs.QUERY_RESULT_SIZE,
+            CommonConstants.CursorConfigs.DEFAULT_QUERY_RESULT_SIZE);
+      }
+
+      if (numRows > CommonConstants.CursorConfigs.MAX_QUERY_RESULT_SIZE) {

Review Comment:
   `MAX_CURSOR_NUM_ROWS` is a better name. There is no restriction on the size 
of the query result. The restriction is on the number of rows in a specific 
fetch. I added this as a backstop to runaway cursor fetch workloads. Couple of 
options:
   * I can remove it.
   * Default to a much larger number. (500K right now).
   
   Preferences? 



-- 
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: commits-unsubscr...@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to