davidm-db commented on code in PR #49427:
URL: https://github.com/apache/spark/pull/49427#discussion_r1928409489
##########
sql/core/src/main/scala/org/apache/spark/sql/scripting/SqlScriptingExecutionContext.scala:
##########
@@ -191,29 +189,32 @@ class SqlScriptingExecutionScope(
*/
def findHandler(condition: String, sqlState: String):
Option[ErrorHandlerExec] = {
// Check if there is a specific handler for the given condition.
- conditionHandlerMap.get(condition)
- .orElse {
- conditionHandlerMap.get(sqlState) match {
- // If SQLSTATE handler is defined, use it only for errors with class
!= '02'.
- case Some(handler) if !sqlState.startsWith("02") => Some(handler)
- case _ => None
- }
- }
- .orElse {
- conditionHandlerMap.get("NOT FOUND") match {
- // If NOT FOUND handler is defined, use it only for errors with
class == '02'.
- case Some(handler) if sqlState.startsWith("02") => Some(handler)
- case _ => None
- }
+ var errorHandler: Option[ErrorHandlerExec] = None
+
+ errorHandler = triggerHandlerMap.getHandlerForCondition(condition)
+
+ if (errorHandler.isEmpty) {
+ // Check if there is a specific handler for the given SQLSTATE.
+ errorHandler = triggerHandlerMap.getHandlerForSqlState(sqlState)
+ }
+
+ if (errorHandler.isEmpty) {
+ errorHandler = triggerHandlerMap.getNotFoundHandler match {
+ case Some(handler) if sqlState.startsWith("02") => Some(handler)
+ case _ => None
}
- .orElse {
- conditionHandlerMap.get("SQLEXCEPTION") match {
- // If SQLEXCEPTION handler is defined, use it only for errors with
class
- // different from 'XX' and '02'.
- case Some(handler) if
- !sqlState.startsWith("XX") && !sqlState.startsWith("02") =>
Some(handler)
- case _ => None
- }
+ }
+
+ if (errorHandler.isEmpty) {
+ // If SQLEXCEPTION handler is defined, use it only for errors with class
+ // different from 'XX' and '02'.
+ errorHandler = triggerHandlerMap.getSqlExceptionHandler match {
+ case Some(handler) if !sqlState.startsWith("XX") &&
!sqlState.startsWith("02") =>
Review Comment:
If we are adding support for case insensitive checks, as Wenchen suggested,
should we add it here as well for the "XX"?
--
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]
