Skip to content

Commit c159033

Browse files
davidm-dbMaxGekk
authored andcommitted
[SPARK-48376][SQL][FOLLOWUP] Add ITERATE statement fixes
### What changes were proposed in this pull request? Previous [pull request](#47973) introduced new logic for `LEAVE` and `ITERATE` statements, but I missed to introduce minor part of `ITERATE` statement handling. While `ITERATE` statement is not allowed for compounds (i.e. user cannot use `LEAVE <label>` where label belongs to compound) it's still possible to use `ITERATE` statement within the compound (i.e. when using label of the surrounding `WHILE` loop for example). In such cases, it's required to drop variables defined in the compound (the same way as for `ITERATE` statement). We don't yet have execution logic (I will work on POC and design doc during the next week), but this is setting up stuff now, so we don't miss anything in future. ### Why are the changes needed? Changes are minor, they improve consistency among test suites for SQL scripting. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Added small test, but not completely related to this change. Since we don't have execution logic yet, it's hard to test. I will add more tests together with the execution logic. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #48022 from davidm-db/missing_logic_for_leave_statement. Authored-by: David Milicevic <[email protected]> Signed-off-by: Max Gekk <[email protected]>
1 parent 2912964 commit c159033

File tree

2 files changed

+59
-1
lines changed

2 files changed

+59
-1
lines changed

sql/core/src/main/scala/org/apache/spark/sql/scripting/SqlScriptingExecutionNode.scala

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,10 @@ abstract class CompoundNestedStatementIteratorExec(
176176
handleLeaveStatement(leaveStatement)
177177
curr = None
178178
leaveStatement
179+
case Some(iterateStatement: IterateStatementExec) =>
180+
handleIterateStatement(iterateStatement)
181+
curr = None
182+
iterateStatement
179183
case Some(statement: LeafStatementExec) =>
180184
curr = if (localIterator.hasNext) Some(localIterator.next()) else None
181185
statement
@@ -185,6 +189,9 @@ abstract class CompoundNestedStatementIteratorExec(
185189
case leaveStatement: LeaveStatementExec =>
186190
handleLeaveStatement(leaveStatement)
187191
leaveStatement
192+
case iterateStatement: IterateStatementExec =>
193+
handleIterateStatement(iterateStatement)
194+
iterateStatement
188195
case other => other
189196
}
190197
} else {
@@ -206,18 +213,35 @@ abstract class CompoundNestedStatementIteratorExec(
206213
stopIteration = false
207214
}
208215

209-
/** Actions to do when LEAVE statement is encountered to stop the execution of this compound. */
216+
/** Actions to do when LEAVE statement is encountered, to stop the execution of this compound. */
210217
private def handleLeaveStatement(leaveStatement: LeaveStatementExec): Unit = {
211218
if (!leaveStatement.hasBeenMatched) {
212219
// Stop the iteration.
213220
stopIteration = true
214221

215222
// TODO: Variable cleanup (once we add SQL script execution logic).
223+
// TODO: Add interpreter tests as well.
216224

217225
// Check if label has been matched.
218226
leaveStatement.hasBeenMatched = label.isDefined && label.get.equals(leaveStatement.label)
219227
}
220228
}
229+
230+
/**
231+
* Actions to do when ITERATE statement is encountered, to stop the execution of this compound.
232+
*/
233+
private def handleIterateStatement(iterateStatement: IterateStatementExec): Unit = {
234+
if (!iterateStatement.hasBeenMatched) {
235+
// Stop the iteration.
236+
stopIteration = true
237+
238+
// TODO: Variable cleanup (once we add SQL script execution logic).
239+
// TODO: Add interpreter tests as well.
240+
241+
// No need to check if label has been matched, since ITERATE statement is already
242+
// not allowed in CompoundBody.
243+
}
244+
}
221245
}
222246

223247
/**

sql/core/src/test/scala/org/apache/spark/sql/scripting/SqlScriptingInterpreterSuite.scala

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -678,4 +678,38 @@ class SqlScriptingInterpreterSuite extends QueryTest with SharedSparkSession {
678678
)
679679
verifySqlScriptResult(sqlScriptText, expected)
680680
}
681+
682+
test("nested compounds in loop - leave in inner compound") {
683+
val sqlScriptText =
684+
"""
685+
|BEGIN
686+
| DECLARE x INT;
687+
| SET x = 0;
688+
| lbl: WHILE x < 2 DO
689+
| SET x = x + 1;
690+
| BEGIN
691+
| SELECT 1;
692+
| lbl2: BEGIN
693+
| SELECT 2;
694+
| LEAVE lbl2;
695+
| SELECT 3;
696+
| END;
697+
| END;
698+
| END WHILE;
699+
| SELECT x;
700+
|END""".stripMargin
701+
val expected = Seq(
702+
Seq.empty[Row], // declare
703+
Seq.empty[Row], // set x = 0
704+
Seq.empty[Row], // set x = 1
705+
Seq(Row(1)), // select 1
706+
Seq(Row(2)), // select 2
707+
Seq.empty[Row], // set x = 2
708+
Seq(Row(1)), // select 1
709+
Seq(Row(2)), // select 2
710+
Seq(Row(2)), // select x
711+
Seq.empty[Row] // drop
712+
)
713+
verifySqlScriptResult(sqlScriptText, expected)
714+
}
681715
}

0 commit comments

Comments
 (0)