dusantism-db commented on code in PR #51595:
URL: https://github.com/apache/spark/pull/51595#discussion_r2222611827
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/SqlScriptingParserSuite.scala:
##########
@@ -2336,6 +2336,120 @@ class SqlScriptingParserSuite extends SparkFunSuite
with SQLHelper {
assert(forStatement.label.get == "lbl_4")
}
+ test("for variable not the same as labels in scope") {
+ val sqlScriptText =
+ """
+ |BEGIN
+ | L1: BEGIN
+ | L2: BEGIN
+ | L3: FOR L4 AS SELECT 1 DO
+ | SELECT 1;
+ | FOR L5 AS SELECT 3 DO
+ | BEGIN
+ | SELECT L4;
+ | END;
+ | SELECT 4;
+ | END FOR;
+ | END FOR L3;
+ | END L2;
+ | L4: BEGIN
+ | SELECT 3;
+ | END L4;
+ | END L1;
+ |END""".stripMargin
+
+ val tree = parsePlan(sqlScriptText).asInstanceOf[CompoundBody]
+ assert(tree.collection.length == 1)
+ assert(tree.collection.head.isInstanceOf[CompoundBody])
+ }
+
+ test("for variable name is the same as a label in scope - should fail") {
+ val sqlScriptText =
+ """
+ |BEGIN
+ | L1: BEGIN
+ | L2: BEGIN
+ | L3: FOR L2 AS SELECT 1 DO
+ | SELECT 1;
+ | SELECT 2;
+ | END FOR L3;
+ | END L2;
+ | L4: BEGIN
+ | SELECT 3;
+ | END L4;
+ | END L1;
+ |END""".stripMargin
+
+ val e = intercept[SqlScriptingException] {
+ parsePlan(sqlScriptText).asInstanceOf[CompoundBody]
+ }
+ assert(e.getCondition ===
"FOR_VARIABLE_NAME_FORBIDDEN.NAME_ALREADY_IN_USE")
+ assert(e.getMessage.contains("An error regarding the naming of an
iterator" +
+ " variable inside a for loop occurred. Variable name "))
+ assert(e.getMessage.contains(" used in FOR statement is already used in
another " +
+ "parenting FOR loop or as a label in parenting scope. This is forbidden
" +
+ "to avoid issues with referencing variables/columns."))
Review Comment:
Have we tried using `checkError` for this? There are examples in this suite
of how it's used.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/SqlScriptingParserSuite.scala:
##########
@@ -2336,6 +2336,120 @@ class SqlScriptingParserSuite extends SparkFunSuite
with SQLHelper {
assert(forStatement.label.get == "lbl_4")
}
+ test("for variable not the same as labels in scope") {
+ val sqlScriptText =
+ """
+ |BEGIN
+ | L1: BEGIN
+ | L2: BEGIN
+ | L3: FOR L4 AS SELECT 1 DO
+ | SELECT 1;
+ | FOR L5 AS SELECT 3 DO
+ | BEGIN
+ | SELECT L4;
+ | END;
+ | SELECT 4;
+ | END FOR;
+ | END FOR L3;
+ | END L2;
+ | L4: BEGIN
+ | SELECT 3;
+ | END L4;
+ | END L1;
+ |END""".stripMargin
+
+ val tree = parsePlan(sqlScriptText).asInstanceOf[CompoundBody]
+ assert(tree.collection.length == 1)
+ assert(tree.collection.head.isInstanceOf[CompoundBody])
+ }
+
+ test("for variable name is the same as a label in scope - should fail") {
+ val sqlScriptText =
+ """
+ |BEGIN
+ | L1: BEGIN
+ | L2: BEGIN
+ | L3: FOR L2 AS SELECT 1 DO
+ | SELECT 1;
+ | SELECT 2;
+ | END FOR L3;
+ | END L2;
+ | L4: BEGIN
+ | SELECT 3;
+ | END L4;
+ | END L1;
+ |END""".stripMargin
+
+ val e = intercept[SqlScriptingException] {
+ parsePlan(sqlScriptText).asInstanceOf[CompoundBody]
+ }
+ assert(e.getCondition ===
"FOR_VARIABLE_NAME_FORBIDDEN.NAME_ALREADY_IN_USE")
+ assert(e.getMessage.contains("An error regarding the naming of an
iterator" +
+ " variable inside a for loop occurred. Variable name "))
+ assert(e.getMessage.contains(" used in FOR statement is already used in
another " +
+ "parenting FOR loop or as a label in parenting scope. This is forbidden
" +
+ "to avoid issues with referencing variables/columns."))
Review Comment:
Same for other tests which should fail
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/SqlScriptingParserSuite.scala:
##########
@@ -2336,6 +2336,120 @@ class SqlScriptingParserSuite extends SparkFunSuite
with SQLHelper {
assert(forStatement.label.get == "lbl_4")
}
+ test("for variable not the same as labels in scope") {
+ val sqlScriptText =
+ """
+ |BEGIN
+ | L1: BEGIN
+ | L2: BEGIN
+ | L3: FOR L4 AS SELECT 1 DO
+ | SELECT 1;
+ | FOR L5 AS SELECT 3 DO
+ | BEGIN
+ | SELECT L4;
+ | END;
+ | SELECT 4;
+ | END FOR;
+ | END FOR L3;
+ | END L2;
+ | L4: BEGIN
+ | SELECT 3;
+ | END L4;
+ | END L1;
+ |END""".stripMargin
+
+ val tree = parsePlan(sqlScriptText).asInstanceOf[CompoundBody]
+ assert(tree.collection.length == 1)
+ assert(tree.collection.head.isInstanceOf[CompoundBody])
+ }
+
+ test("for variable name is the same as a label in scope - should fail") {
+ val sqlScriptText =
+ """
+ |BEGIN
+ | L1: BEGIN
+ | L2: BEGIN
+ | L3: FOR L2 AS SELECT 1 DO
+ | SELECT 1;
+ | SELECT 2;
+ | END FOR L3;
+ | END L2;
+ | L4: BEGIN
+ | SELECT 3;
+ | END L4;
+ | END L1;
+ |END""".stripMargin
+
+ val e = intercept[SqlScriptingException] {
+ parsePlan(sqlScriptText).asInstanceOf[CompoundBody]
+ }
+ assert(e.getCondition ===
"FOR_VARIABLE_NAME_FORBIDDEN.NAME_ALREADY_IN_USE")
+ assert(e.getMessage.contains("An error regarding the naming of an
iterator" +
+ " variable inside a for loop occurred. Variable name "))
+ assert(e.getMessage.contains(" used in FOR statement is already used in
another " +
+ "parenting FOR loop or as a label in parenting scope. This is forbidden
" +
+ "to avoid issues with referencing variables/columns."))
+ }
+
+ test("for variable name is the same as the label of the for loop - should
fail") {
+ val sqlScriptText =
+ """
+ |BEGIN
+ | L1: FOR L1 AS SELECT 1 DO
+ | SELECT 2;
+ | END FOR L1;
+ |END""".stripMargin
+
+ val e = intercept[SqlScriptingException] {
+ parsePlan(sqlScriptText).asInstanceOf[CompoundBody]
+ }
+ assert(e.getCondition ===
"FOR_VARIABLE_NAME_FORBIDDEN.NAME_ALREADY_IN_USE")
+ assert(e.getMessage.contains("An error regarding the naming of an
iterator" +
+ " variable inside a for loop occurred. Variable name "))
+ assert(e.getMessage.contains(" used in FOR statement is already used in
another " +
+ "parenting FOR loop or as a label in parenting scope. This is forbidden
" +
+ "to avoid issues with referencing variables/columns."))
+ }
+
+ test("label name is the same as the for loop variable name - should fail") {
+ val sqlScriptText =
+ """
+ |BEGIN
+ | FOR L1 AS SELECT 1 DO
+ | L1: BEGIN
+ | SELECT 2;
+ | END L1;
+ | END FOR;
+ |END""".stripMargin
+ val e = intercept[SqlScriptingException] {
+ parsePlan(sqlScriptText).asInstanceOf[CompoundBody]
+ }
+ assert(e.getCondition === "LABEL_OR_FOR_VARIABLE_ALREADY_EXISTS")
+ assert(e.getMessage.contains("The label or for variable "))
+ assert(e.getMessage.contains(" already exists. Choose another " +
+ "name or rename the existing label."))
+ }
+
+ test("label name is the same as the for loop variable - should fail") {
+ val sqlScriptText =
+ """
+ |BEGIN
+ | FOR L1 AS SELECT 1 DO
+ | L1: BEGIN
+ | SELECT 2;
+ | END L1;
+ | END FOR;
+ |END""".stripMargin
Review Comment:
This test is the same as the one above?
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
