abhishekagarwal87 commented on a change in pull request #12033:
URL: https://github.com/apache/druid/pull/12033#discussion_r764614328
##########
File path: sql/src/main/java/org/apache/druid/sql/http/SqlResource.java
##########
@@ -205,22 +205,20 @@ public Response doPost(
throw (ForbiddenException)
serverConfig.getErrorResponseTransformStrategy()
.transformIfNeeded(e); // let
ForbiddenExceptionMapper handle this
}
+ catch (RelOptPlanner.CannotPlanException e) {
+ endLifecycle(sqlQueryId, lifecycle, e, remoteAddr, -1);
+ SqlPlanningException spe = new
SqlPlanningException(SqlPlanningException.PlanningError.UNSUPPORTED_SQL_ERROR,
+ e.getMessage());
+ return buildNonOkResponse(BadQueryException.STATUS_CODE, spe,
sqlQueryId);
Review comment:
I think it is weird currently that the HTTP error code that the user
gets depends on when can we detect that the query is bad or unsupported. I
wanted the behaviour to be consistent irrespective of the stage, the query gets
rejected.
If there is an unknown failure (NPE, ISE) during the detection path, that
should be an internal server error but if we did detect it correctly that query
is unplannable then we know we can't support the said query syntax. this is why
400 is returned only in the case of CannotPlanException.
Like a user trying to do max aggregation on a string or a user trying to use
multiple exact distinct counts will get 500 right now even though it is
documented that these features are not supported. these are indeed bad queries
in that sense. Also, what matters more than the error code, is the info we are
giving alongside in the response since that is more useful for the end-user.
--
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]
