[SPARK-48353][FOLLOW UP][SQL] Exception handling improvements #51034
+696
−111
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
This pull request proposes an improvement in how conditions are matched with exception handlers. Previously, condition would match with an exception handler only if the match was full/complete - i.e. condition
<MAIN>.<SUBCLASS>
would match only to the handler defined for<MAIN>.<SUBCLASS>
. With this improvement,<MAIN>.<SUBCLASS>
condition would match to the handlers defined for<MAIN>
condition as well, with correct precedence.This pull requests also proposes a number of fixes for different missing things:
CompoundBodyExec.reset()
is not resetting thescopeStatus
field.LEAVE
statement, after theEXIT
handler has been executed, the logic considered onlyCompoundBodyExec
nodes, whereas it should have included all non-leaf statements.LEAVE
statement is not recognized properly.LEAVE
statement is not recognized properly.hasNext()
inForStatementExec
is executing the query. This causes the issues in cases whenFOR
is the first statement in the compound and the query fails. It means that exception would happen during thehasNext()
checks instead of the actual iteration. In such case, the parent compound (of theFOR
statement) is not entered (because that happens during the iteration) and call stack is not properly setup for exception handling.Changes corresponding to this problems, in the same order:
scopeStatus
field inCompoundBodyExec.reset()
.reset()
on handler before starting its execution inSqlScriptingExecution.handleException()
.curr
pointer toNonLeafStatementExec
and use that inSqlScriptingExecution. getNextStatement()
in case whenEXIT
handler is finished and removed from the stack.LeaveStatementExec
in*.Condition
cases in all of the relevant statement types. When exception happens during the condition execution, theLeaveStatementExec
is injected intocurr
field, but the state hasn't changed. This means that when theLEAVE
statement is to be executed, the state would correspond to the condition. I don't know how to do this better, so any suggestions are more than welcome!LeaveStatementExec
in*Body
cases of if/else and searched case statement - equivalent to the previous bullet.ForStatementExec.hasNext()
.Why are the changes needed?
These changes are fixing wrong logic and improving some of the already existing exception handling mechanisms.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
New unit tests are added for to test/guard all of the introduced improvements.
Was this patch authored or co-authored using generative AI tooling?
No.