You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/04/20 18:20:13 UTC

[GitHub] [pinot] mcvsubbu commented on a diff in pull request #8550: Add query option to use more replica groups

mcvsubbu commented on code in PR #8550:
URL: https://github.com/apache/pinot/pull/8550#discussion_r854428109


##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java:
##########
@@ -2151,6 +2152,18 @@ static void validateRequest(PinotQuery pinotQuery, int queryResponseLimit) {
         || !QueryOptionsUtils.isResponseFormatSQL(queryOptions)) {
       throw new IllegalStateException("SQL query should always have response format and group-by mode set to SQL");
     }
+    try {
+    // throw errors if options is less than 1 or invalid
+      Integer numReplicaGroupsToQuery = QueryOptionsUtils.getNumReplicaGroupsToQuery(queryOptions);
+      if (numReplicaGroupsToQuery != null) {
+        Preconditions.checkState(numReplicaGroupsToQuery > 0, "numReplicaGroups must be "

Review Comment:
   If the user provided input is bad, it should result in a 4xx error. Preconditions check will result in a 5xx. Please fix it.



-- 
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-unsubscribe@pinot.apache.org

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


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