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: [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]
