kgyrtkirk commented on code in PR #15543:
URL: https://github.com/apache/druid/pull/15543#discussion_r1426200050
##########
processing/src/main/java/org/apache/druid/math/expr/FunctionalExpr.java:
##########
@@ -190,11 +191,22 @@ public ExprEval eval(ObjectBinding bindings)
try {
return function.apply(args, bindings);
}
- catch (DruidException | ExpressionValidationException e) {
+ catch (ExpressionValidationException e) {
+ // ExpressionValidationException already contain function name
+ throw DruidException.forPersona(DruidException.Persona.USER)
+ .ofCategory(DruidException.Category.INVALID_INPUT)
+ .build(e.getMessage());
+ }
+ catch (Types.InvalidCastException | Types.InvalidCastBooleanException e) {
+ throw DruidException.forPersona(DruidException.Persona.USER)
+ .ofCategory(DruidException.Category.INVALID_INPUT)
+ .build("Function[%s] encountered exception: %s",
name, e.getMessage());
Review Comment:
okay - I don't have any big expectation from these things ; just 1: if there
was a parent exception it should be set for the new exception ; please restore
that behavior somehow - if the exception will get logged for some reason that
part must also be logged; this is also usefull when a test fails for whatever
reason: having the full stacktrace will make it possible to navigate to the
place where the issue happened.
I think `DruidException` is designed oddly - but its still possible to pass
the parent exception to the `build` method (I think the other method should be
dropped - so that when it gets invoked the developer must fill it to `null` to
get that behaviour - which is less errorprone)
--
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]
