davidm-db commented on code in PR #53894:
URL: https://github.com/apache/spark/pull/53894#discussion_r2716305968
##########
sql/core/src/main/scala/org/apache/spark/sql/scripting/SqlScriptingExecutionNode.scala:
##########
@@ -532,7 +532,10 @@ class WhileStatementExec(
private lazy val treeIterator: Iterator[CompoundStatementExec] =
new Iterator[CompoundStatementExec] {
- override def hasNext: Boolean = !interrupted && curr.nonEmpty
+ override def hasNext: Boolean = !interrupted && curr.nonEmpty && (state
match {
+ case WhileState.Body => body.getTreeIterator.hasNext
+ case WhileState.Condition => curr.nonEmpty
+ })
Review Comment:
This change is going to have a bug, the example is:
```
BEGIN
DECLARE i INT DEFAULT 0;
DECLARE s INT DEFAULT 0;
DECLARE ch_sum INT DEFAULT 0;
DECLARE CONTINUE HANDLER FOR DIVIDE_BY_ZERO SET ch_sum = ch_sum + 1;
WHILE i < 5 DO
SET i = i + 1;
IF (1/0)==0 THEN
SELECT 100;
END IF;
END WHILE;
SELECT i, s as sum, ch_sum;
END
```
This loop will have only one iteration, because the IF is the last nested
statement in the WHILE's body, the `hasNext` check like this will fail because
`body.getTreeIterator.hasNext` will return `false`, which will prevent us from
moving to the condition of the WHILE, and thus to the next iteration.
It looks like this is not needed at all, but let's test without it and see
is anything is failing and then figure out a better way to do it.
--
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]
