davidm-db commented on code in PR #46665:
URL: https://github.com/apache/spark/pull/46665#discussion_r1625787748


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -116,6 +116,57 @@ class AstBuilder extends DataTypeAstBuilder with 
SQLConfHelper with Logging {
     }
   }
 
+  override def visitCompoundOrSingleStatement(
+      ctx: CompoundOrSingleStatementContext): CompoundBody = {
+    if (ctx.singleCompound() != null) {
+      visit(ctx.singleCompound()).asInstanceOf[CompoundBody]
+    } else {
+      val logicalPlan = visitSingleStatement(ctx.singleStatement())
+      CompoundBody(List(SparkStatementWithPlan(
+        parsedPlan = logicalPlan,
+        sourceStart = ctx.start.getStartIndex,
+        sourceEnd = ctx.stop.getStopIndex + 1)))
+    }
+  }
+
+  override def visitSingleCompound(ctx: SingleCompoundContext): CompoundBody = 
{
+    visit(ctx.beginEndCompoundBlock()).asInstanceOf[CompoundBody]
+  }
+
+  private def visitCompoundBodyImpl(ctx: CompoundBodyContext): CompoundBody = {
+    val buff = ListBuffer[CompoundPlanStatement]()
+    for (i <- 0 until ctx.getChildCount) {
+      val child = ctx.getChild(i)
+      val visitedChild = visit(ctx.getChild(i))
+      visitedChild match {
+        case statement: CompoundPlanStatement => buff += statement
+        case null if child.isInstanceOf[TerminalNodeImpl] =>

Review Comment:
   Well, I agree. We had only `case null =>` but it was very unclear what it 
meant, so we decided to add this check. 
   
   This way, if anything changes in the future, we would get exceptions in our 
tests if this `null` does not happen because the `child` is instance of 
`TerminalNodeImpl`.
   
   Not sure if this is the best way to do it though. I guess that it's probably 
better to not visit `child` at all if it's a terminal node. I don't know how 
better to achieve this. Or maybe we are fine with leaving only `case null =>` 
and adding comment that explains 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: 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

Reply via email to