jineshparakh commented on code in PR #18515:
URL: https://github.com/apache/pinot/pull/18515#discussion_r3271584716
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java:
##########
@@ -219,29 +230,40 @@ public void processSqlQueryPost(String query, @Suspended
AsyncResponse asyncResp
+ "Supports both single-stage and multi-stage queries.")
@ApiResponses(value = {
@ApiResponse(code = 200, message = "Query fingerprint"),
+ @ApiResponse(code = 400, message = "Bad Request"),
@ApiResponse(code = 500, message = "Internal Server Error")
})
@ManualAuthorization
public Response getQueryFingerprint(String query,
@Context org.glassfish.grizzly.http.server.Request requestContext,
@Context HttpHeaders httpHeaders) {
try {
- JsonNode requestJson = JsonUtils.stringToJsonNode(query);
+ JsonNode requestJson;
+ try {
+ requestJson = JsonUtils.stringToJsonNode(query);
+ } catch (JsonProcessingException e) {
+ throw new BadRequestException("Invalid JSON: " + e.getMessage(), e);
+ }
if (!requestJson.has(Request.SQL)) {
- throw new IllegalStateException("Payload is missing the query string
field 'sql'");
+ throw new BadRequestException("Payload is missing the query string
field 'sql'");
}
QueryFingerprint fingerprint = generateQueryFingerprint((ObjectNode)
requestJson);
return Response.ok(fingerprint).build();
+ } catch (BadRequestException bre) {
+ _brokerMetrics.addMeteredGlobalValue(BrokerMeter.BAD_REQUEST_EXCEPTIONS,
1L);
+ throw bre;
Review Comment:
The error response inconsistency is pre-existing and the response structure
is different for every API.
Fixing them all will bring up another backward incompatibility.
Do you think this should be done?
--
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]
