gortiz commented on code in PR #15533:
URL: https://github.com/apache/pinot/pull/15533#discussion_r2042148147


##########
pinot-spi/src/main/java/org/apache/pinot/spi/exception/QueryErrorCode.java:
##########
@@ -57,6 +57,8 @@ public enum QueryErrorCode {
   UNKNOWN_COLUMN(710, "UnknownColumnError"),
   ///  Error while planning the query. For example, trying to run a colocated 
join on non-colocated tables.
   QUERY_PLANNING(720, "QueryPlanningError"),
+  ///  Query already errored out.
+  ERRORED_OUT(800, "ErroredOutError"),

Review Comment:
   I don't think this belongs here. This class represents the error codes sent 
to the user, but this literal looks like a new value you are adding just to 
know whether the exception was logged or not. That is not the responsibility of 
the error code class.
   
   I'm not sure why you need to know if the error was logged or not, but in 
case you do, it may be a responsibility of the QueryException, but never of the 
ErrorCode, so maybe you can add a boolean attribute to that class



-- 
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

Reply via email to