agavra commented on code in PR #9818:
URL: https://github.com/apache/pinot/pull/9818#discussion_r1026636713
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java:
##########
@@ -129,6 +130,9 @@ public QueryPlan planQuery(String sqlQuery,
SqlNodeAndOptions sqlNodeAndOptions)
plannerContext.setOptions(sqlNodeAndOptions.getOptions());
RelRoot relRoot = compileQuery(sqlNodeAndOptions.getSqlNode(),
plannerContext);
return toDispatchablePlan(relRoot, plannerContext);
+ } catch (CalciteContextException e) {
+ throw new RuntimeException("Error composing query plan for '" + sqlQuery
+ + "': " + e.getMessage() + "'", e);
Review Comment:
the problem is that what's shown to the user isn't the entire stack trace,
that part gets swallowed upstream somewhere (and that makes sense, that would
make for a bad user experience to see the entire stack trace and causedBy). All
they see is what's in the error message.
> or the other way to ask is why can't we add e.getMessage() to both
branches of catch exception?
that's what I was saying earlier. I don't want to add the cause if the
exception makes no sense (like a `NullPointerException`) - the user shouldn't
see that kind of message as it really isn't helpful.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
