ankitsultana commented on code in PR #11437:
URL: https://github.com/apache/pinot/pull/11437#discussion_r1323500172
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java:
##########
@@ -242,19 +246,23 @@ public BrokerResponse handleRequest(JsonNode request,
@Nullable SqlNodeAndOption
throws Exception {
requestContext.setRequestArrivalTimeMillis(System.currentTimeMillis());
+ long requestId = _brokerIdGenerator.get();
+ requestContext.setRequestId(requestId);
+
// First-stage access control to prevent unauthenticated requests from
using up resources. Secondary table-level
// check comes later.
boolean hasAccess =
_accessControlFactory.create().hasAccess(requesterIdentity);
if (!hasAccess) {
_brokerMetrics.addMeteredGlobalValue(BrokerMeter.REQUEST_DROPPED_DUE_TO_ACCESS_ERROR,
1);
requestContext.setErrorCode(QueryException.ACCESS_DENIED_ERROR_CODE);
+ _brokerQueryEventListener.onQueryCompletion(new
BrokerQueryEventInfo(requestContext));
throw new WebApplicationException("Permission denied",
Response.Status.FORBIDDEN);
}
- long requestId = _brokerIdGenerator.get();
- requestContext.setRequestId(requestId);
JsonNode sql = request.get(Broker.Request.SQL);
if (sql == null) {
+
requestContext.setErrorCode(QueryException.BROKER_REQUEST_SEND_ERROR_CODE);
Review Comment:
`BROKER_REQUEST_SEND_ERROR_CODE` at present is getting used at two places:
1. For multistage queries it is returned when brokerTenantsUnion is empty
(in controller's PinotQueryResource)
2. For v1 queries it is returned if there was an error in dispatching
queries to servers (based on a cursory reading of the code. I might be wrong)
I don't think if the user is missing the "sql" in their request payload we
should be using this error code. Maybe `SQL_PARSING_ERROR_CODE` would be more
appropriate.
Other reviewers can probably chime in and suggest something better.
--
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]
