miland-db commented on code in PR #53530:
URL: https://github.com/apache/spark/pull/53530#discussion_r2690596895
##########
sql/core/src/main/scala/org/apache/spark/sql/scripting/SqlScriptingExecution.scala:
##########
@@ -112,6 +112,82 @@ class SqlScriptingExecution(
}
}
+ /**
+ * Helper method to set the returnCurrentWithoutAdvancing flag on
CompoundBodyExec.
+ * After a CONTINUE handler, the next statement to execute is already in
curr,
+ * so we shouldn't advance the iterator.
+ * @param executionPlan Execution plan.
+ */
+ private def setReturnCurrentWithoutAdvancing(executionPlan:
NonLeafStatementExec): Unit = {
+ // We need to find the innermost CompoundBodyExec that's currently
executing
+ // Navigate through the execution plan following curr pointers
+ var currExecPlan: NonLeafStatementExec = executionPlan
+
+ // Keep going deeper while we have non-leaf statements
+ var foundTarget = false
+ while (!foundTarget && currExecPlan.curr.isDefined) {
+ currExecPlan.curr.get match {
+ case compound: CompoundBodyExec =>
+ // Found a CompoundBodyExec - set the flag and try to go deeper
+ compound.returnCurrentWithoutAdvancing = true
+ foundTarget = true
+ currExecPlan = compound
+
+ case loop: RepeatStatementExec =>
+ // For REPEAT, navigate to its body
+ loop.curr match {
+ case Some(body: CompoundBodyExec) =>
+ body.returnCurrentWithoutAdvancing = true
+ foundTarget = true
+ case _ => foundTarget = true // Stop if no body
+ }
+
+ case loop: WhileStatementExec =>
+ // For WHILE, navigate to its body
+ loop.curr match {
+ case Some(body: CompoundBodyExec) =>
+ body.returnCurrentWithoutAdvancing = true
+ foundTarget = true
+ case _ => foundTarget = true
+ }
+
+ case loop: ForStatementExec =>
+ // For FOR, the body is in the statements list
+ // Set flag on all CompoundBodyExec children
+ loop.curr match {
+ case Some(body: CompoundBodyExec) =>
+ body.returnCurrentWithoutAdvancing = true
+ foundTarget = true
+ case _ => foundTarget = true
+ }
+
+ case ifElse: IfElseStatementExec =>
+ // For IF, navigate to current branch being executed
+ ifElse.curr match {
+ case Some(body: CompoundBodyExec) =>
+ body.returnCurrentWithoutAdvancing = true
+ foundTarget = true
+ case _ => foundTarget = true
+ }
+
+ case other: NonLeafStatementExec =>
+ // Other non-leaf, try to go deeper
+ currExecPlan = other
+
+ case _ =>
+ // Leaf statement, stop here
+ foundTarget = true
+ }
+ }
+
+ // Also set on the top-level if it's a CompoundBodyExec
+ executionPlan match {
+ case compound: CompoundBodyExec =>
+ compound.returnCurrentWithoutAdvancing = true
+ case _ => // pass
+ }
+ }
+
Review Comment:
I am not sure we need this after this fix:
https://github.com/apache/spark/pull/53759
Please merge latest changes and test cursors without this flag
`returnCurrentWithoutAdvancing`
##########
sql/core/src/main/scala/org/apache/spark/sql/scripting/SqlScriptingExecution.scala:
##########
@@ -112,6 +112,82 @@ class SqlScriptingExecution(
}
}
+ /**
+ * Helper method to set the returnCurrentWithoutAdvancing flag on
CompoundBodyExec.
+ * After a CONTINUE handler, the next statement to execute is already in
curr,
+ * so we shouldn't advance the iterator.
+ * @param executionPlan Execution plan.
+ */
+ private def setReturnCurrentWithoutAdvancing(executionPlan:
NonLeafStatementExec): Unit = {
+ // We need to find the innermost CompoundBodyExec that's currently
executing
+ // Navigate through the execution plan following curr pointers
+ var currExecPlan: NonLeafStatementExec = executionPlan
+
+ // Keep going deeper while we have non-leaf statements
+ var foundTarget = false
+ while (!foundTarget && currExecPlan.curr.isDefined) {
+ currExecPlan.curr.get match {
+ case compound: CompoundBodyExec =>
+ // Found a CompoundBodyExec - set the flag and try to go deeper
+ compound.returnCurrentWithoutAdvancing = true
+ foundTarget = true
+ currExecPlan = compound
+
+ case loop: RepeatStatementExec =>
+ // For REPEAT, navigate to its body
+ loop.curr match {
+ case Some(body: CompoundBodyExec) =>
+ body.returnCurrentWithoutAdvancing = true
+ foundTarget = true
+ case _ => foundTarget = true // Stop if no body
+ }
+
+ case loop: WhileStatementExec =>
+ // For WHILE, navigate to its body
+ loop.curr match {
+ case Some(body: CompoundBodyExec) =>
+ body.returnCurrentWithoutAdvancing = true
+ foundTarget = true
+ case _ => foundTarget = true
+ }
+
+ case loop: ForStatementExec =>
+ // For FOR, the body is in the statements list
+ // Set flag on all CompoundBodyExec children
+ loop.curr match {
+ case Some(body: CompoundBodyExec) =>
+ body.returnCurrentWithoutAdvancing = true
+ foundTarget = true
+ case _ => foundTarget = true
+ }
+
+ case ifElse: IfElseStatementExec =>
+ // For IF, navigate to current branch being executed
+ ifElse.curr match {
+ case Some(body: CompoundBodyExec) =>
+ body.returnCurrentWithoutAdvancing = true
+ foundTarget = true
+ case _ => foundTarget = true
+ }
+
+ case other: NonLeafStatementExec =>
+ // Other non-leaf, try to go deeper
+ currExecPlan = other
+
+ case _ =>
+ // Leaf statement, stop here
+ foundTarget = true
+ }
+ }
+
+ // Also set on the top-level if it's a CompoundBodyExec
+ executionPlan match {
+ case compound: CompoundBodyExec =>
+ compound.returnCurrentWithoutAdvancing = true
+ case _ => // pass
+ }
+ }
+
Review Comment:
I am not sure we need this code after this fix:
https://github.com/apache/spark/pull/53759
Please merge latest changes and test cursors without this flag
`returnCurrentWithoutAdvancing`
--
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]
