davidm-db commented on code in PR #47026: URL: https://github.com/apache/spark/pull/47026#discussion_r1664251369
########## sql/core/src/main/scala/org/apache/spark/sql/scripting/SqlScriptingExecutionNode.scala: ########## @@ -0,0 +1,147 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql.scripting + +import org.apache.spark.SparkException +import org.apache.spark.internal.Logging +import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan +import org.apache.spark.sql.catalyst.trees.{Origin, WithOrigin} + +/** + * Trait for all SQL scripting execution nodes used during interpretation phase. + */ +sealed trait CompoundStatementExec extends Logging { + + /** + * Whether the statement originates from the SQL script or is created during the interpretation. + * Example: DropVariable statements are automatically created at the end of each compound. + */ + val isInternal: Boolean = false + + /** + * Reset execution of the current node. + */ + def reset(): Unit +} + +/** + * Leaf node in the execution tree. + */ +trait LeafStatementExec extends CompoundStatementExec + +/** + * Non-leaf node in the execution tree. It is an iterator over executable child nodes. + */ +trait NonLeafStatementExec extends CompoundStatementExec with Iterator[CompoundStatementExec] Review Comment: It is different and here is the explanation - we didn't inherit `TreeNode` since interpreter does not go through regular execution path. Intended use of the interpreter is like: - Call interpreter's `buildExecutionPlan` with the parser output plan. - It will return iterator over executable statements. - Use something as simple as `foreach`, `flatMap`, etc. and leverage the interpreter interface that has already been implemented and that will iterate through the statements. - Alternately, for debugging feature, iterator is especially useful because we can operate on it directly based on the debugging commands. If we were to introduce exec nodes that inherit `TreeNode`, this would introduce another layer, because we would need to implement interpreter logic outside of these classes. Another reason why having `children` of executable nodes might be a bit ambiguous approach is in cases when we have loops for example - we cannot know in advance what would be the number of statements to be executed. Does this make sense to you? -- 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