jineshparakh commented on code in PR #18515:
URL: https://github.com/apache/pinot/pull/18515#discussion_r3271593592
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java:
##########
@@ -472,10 +520,15 @@ public void
processSqlQueryWithBothEnginesAndCompareResults(String query, @Suspe
CompletableFuture.allOf(v1Response, v2Response).join();
asyncResponse.resume(getPinotQueryComparisonResponse(v1Query,
v1Response.get(), v2Response.get()));
+ } catch (BadRequestException bre) {
Review Comment:
`executeSqlQuery` is a private method that never throws BRE. It returns
errors via `BrokerResponseNative` (e.g., return new
BrokerResponseNative(QueryErrorCode.SQL_PARSING, e.getMessage())),
not by throwing. So no BRE can originate from inside the supplyAsync
lambdas.
The BRE catch only catches errors from the synchronous JSON parsing and
field validation that runs before the CompletableFutures are created:
1. JsonUtils.stringToJsonNode(query) → JPE → BRE (nested try)
2. Missing sql/v1sql/v2sql field checks → BRE
Both execute before any supplyAsync call, so the CompletionException
wrapping doesn't apply.
--
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]
