gortiz commented on code in PR #15073:
URL: https://github.com/apache/pinot/pull/15073#discussion_r1986859888
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java:
##########
@@ -310,139 +310,64 @@ protected BrokerResponse handleRequest(long requestId,
String query, SqlNodeAndO
}
}
+ /**
+ * CompileResult holds the result of the compilation phase. Compilation may
or may not be successful. If compilation
+ * is successful then all member variables other than BrokerResponse will be
available. If compilation is not
+ * successful, then only the BrokerResponse is set. This is done to keep the
current behaviour as is.
+ * It became hard to keep the current behaviour if we were to throw an
exception from the compileRequest method.
+ * The only exception is that a BrokerResponse is returned for a
literal-only query.
+ */
Review Comment:
My recommendation in cases like this is to use an `Either` pattern where we
have as base class and two subclasses. The Right one with the success case and
the Left where BrokerResponse is set. That requires a bit more code, but we get
type safety in return.
--
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
