[WIP][SPARK-53621][CORE] Adding Support for Executing CONTINUE HANDLER #52371
Conversation
… as a continue handler feature switch;
…onf.scala Co-authored-by: Wenchen Fan <cloud0fan@gmail.com>
…onf.scala Co-authored-by: Wenchen Fan <cloud0fan@gmail.com>
…er/SqlScriptingParserSuite.scala Co-authored-by: Wenchen Fan <cloud0fan@gmail.com>
…ptingInterpreterSuite.scala Co-authored-by: Wenchen Fan <cloud0fan@gmail.com>
TeodorDjelic
changed the title
TeodorDjelic
changed the title
TeodorDjelic
changed the title
TeodorDjelic
changed the title
| val handlerScopeLabel = if (handler.handlerType == ExceptionHandlerType.EXIT | ||
| || handler.handlerType == ExceptionHandlerType.CONTINUE) { |
There was a problem hiding this comment.
I don't get this change. We either leave the if as is or we remove the condition, as these are the only 2 possible handler types.
Not sure if we need this label in a CONTINUE handler though, cc @miland-db what is this label used for again?
There was a problem hiding this comment.
It's the label of the scope where exception handler is defined. @TeodorDjelic as you go through code feel free to add even more comments explaining what's going on.
There was a problem hiding this comment.
This looks like a great place for it.
...server/src/test/scala/org/apache/spark/sql/connect/service/SparkConnectServiceE2ESuite.scala
Outdated
Show resolved
Hide resolved
| if (handler.handlerType == ExceptionHandlerType.CONTINUE) { | ||
| SqlScriptingFrameType.CONTINUE_HANDLER | ||
| } else { | ||
| SqlScriptingFrameType.EXIT_HANDLER | ||
| }, |
There was a problem hiding this comment.
IMO this ExceptionHandlerType.EXIT/CONTINUE and SqlScriptingFrameType.EXIT_HANDLER/CONTINUE_HANDLER is a bit confusing. We should maybe find a way to reuse information about handler type in execution frame. Let's discuss this offline first.
What changes were proposed in this pull request?
This PR implements support for interpreting and executing
CONTINUE HANDLER.Feature is under a new feature switch spark.sql.scripting.continueHandlerEnabled inside
SQLConfig.scala.Why are the changes needed?
This is a part of PRs focused on an effort to add support for
CONTINUE HANDLERs. Follow-up PR will contain more tests.Does this PR introduce any user-facing change?
No.
How was this patch tested?
Yes, an end to end test inside
SqlScriptingE2eSuite.scalawas added testing theCONTINUE HANDLER.Was this patch authored or co-authored using generative AI tooling?
No.