gortiz commented on code in PR #15037:
URL: https://github.com/apache/pinot/pull/15037#discussion_r1975619798
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/combine/BaseCombineOperator.java:
##########
@@ -190,16 +191,21 @@ protected static RuntimeException
wrapOperatorException(Operator operator, Runti
}
// Otherwise, try to get the segment name to help locate the segment when
debugging query errors.
// Not all operators have associated segment, so do this at best effort.
- IndexSegment segment = operator.getIndexSegment();
- String errorMessage = null;
- if (segment != null) {
- errorMessage = "Caught exception while doing operator: " +
operator.getClass()
- + " on segment: " + segment.getSegmentName();
+ // TODO: Do not use class name here but the operator name explain plan. To
do so, that method must be moved from
+ // multi and single stage operators to the base operator
+ String errorMessage = operator.getIndexSegment() != null
+ ? "Caught exception while doing operator: " + operator.getClass()
+ + " on segment " + operator.getIndexSegment().getSegmentName()
+ : "Caught exception while doing operator: " + operator.getClass();
+
+ QueryErrorCode errorCode;
+ if (e instanceof QueryException) {
+ QueryException queryException = (QueryException) e;
+ errorCode = queryException.getErrorCode();
+ } else {
+ errorCode = QueryErrorCode.QUERY_EXECUTION;
}
-
- if (e instanceof IllegalArgumentException || e instanceof
BadQueryRequestException) {
- throw new BadQueryRequestException(errorMessage, e);
- }
- throw new RuntimeException(errorMessage, e);
+ // TODO: Only include exception message if it is a QueryException.
Otherwise, it might expose sensitive information
+ throw errorCode.asException(errorMessage + ": " + e.getMessage(), e);
Review Comment:
this is a strange method for sure. But it seems that only early terminate
exceptions are kept as they are. All others were and are wrapped into another
exception. The only difference in the new code is that they are always a
QueryException now.
I remember I applied some changes to this function in the original PR.
Anyway, in the following PRs we are going to dedicate some time to understand
why it does what it does
--
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
