[SPARK-34701][SQL][FOLLOW-UP] Children/innerChildren should be mutual…

…ly exclusive for AnalysisOnlyCommand

### What changes were proposed in this pull request?

This is a follow up to #32032 (comment). Basically, `children`/`innerChildren` should be mutually exclusive for `AlterViewAsCommand` and `CreateViewCommand`, which extend `AnalysisOnlyCommand`. Otherwise, there could be an issue in the `EXPLAIN` command. Currently, this is not an issue, because these commands will be analyzed (children will always be empty) when the `EXPLAIN` command is run.

### Why are the changes needed?

To be future-proof where these commands are directly used.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Added new tsts

Closes #32447 from imback82/SPARK-34701-followup.

Authored-by: Terry Kim <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/Command.scala

@@ -18,6 +18,7 @@
 package org.apache.spark.sql.catalyst.plans.logical
 
 import org.apache.spark.sql.catalyst.expressions.{Attribute, AttributeSet}
+import org.apache.spark.sql.catalyst.plans.QueryPlan
 import org.apache.spark.sql.catalyst.trees.{BinaryLike, LeafLike, UnaryLike}
 
 /**
@@ -46,5 +47,6 @@ trait AnalysisOnlyCommand extends Command {
   val isAnalyzed: Boolean
   def childrenToAnalyze: Seq[LogicalPlan]
   override final def children: Seq[LogicalPlan] = if (isAnalyzed) Nil else childrenToAnalyze
+  override def innerChildren: Seq[QueryPlan[_]] = if (isAnalyzed) childrenToAnalyze else Nil
   def markAsAnalyzed(): LogicalPlan
 }

sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala

@@ -28,7 +28,6 @@ import org.apache.spark.sql.catalyst.{FunctionIdentifier, SQLConfHelper, TableId
 import org.apache.spark.sql.catalyst.analysis.{GlobalTempView, LocalTempView, PersistedView, ViewType}
 import org.apache.spark.sql.catalyst.catalog.{CatalogStorageFormat, CatalogTable, CatalogTableType, SessionCatalog, TemporaryViewRelation}
 import org.apache.spark.sql.catalyst.expressions.{Alias, Attribute, SubqueryExpression, UserDefinedExpression}
-import org.apache.spark.sql.catalyst.plans.QueryPlan
 import org.apache.spark.sql.catalyst.plans.logical.{AnalysisOnlyCommand, LogicalPlan, Project, View}
 import org.apache.spark.sql.catalyst.util.CharVarcharUtils
 import org.apache.spark.sql.connector.catalog.CatalogV2Implicits.NamespaceHelper
@@ -77,8 +76,6 @@ case class CreateViewCommand(
     copy(plan = newChildren.head)
   }
 
-  override def innerChildren: Seq[QueryPlan[_]] = Seq(plan)
-
   // `plan` needs to be analyzed, but shouldn't be optimized so that caching works correctly.
   override def childrenToAnalyze: Seq[LogicalPlan] = plan :: Nil
 
@@ -256,8 +253,6 @@ case class AlterViewAsCommand(
     copy(query = newChildren.head)
   }
 
-  override def innerChildren: Seq[QueryPlan[_]] = Seq(query)
-
   override def childrenToAnalyze: Seq[LogicalPlan] = query :: Nil
 
   def markAsAnalyzed(): LogicalPlan = copy(isAnalyzed = true)

sql/core/src/test/scala/org/apache/spark/sql/execution/command/PlanResolutionSuite.scala

@@ -30,7 +30,7 @@ import org.apache.spark.sql.catalyst.analysis.{AnalysisTest, Analyzer, EmptyFunc
 import org.apache.spark.sql.catalyst.catalog.{BucketSpec, CatalogStorageFormat, CatalogTable, CatalogTableType, InMemoryCatalog, SessionCatalog}
 import org.apache.spark.sql.catalyst.expressions.{AttributeReference, EqualTo, Expression, InSubquery, IntegerLiteral, ListQuery, Literal, StringLiteral}
 import org.apache.spark.sql.catalyst.parser.{CatalystSqlParser, ParseException}
-import org.apache.spark.sql.catalyst.plans.logical.{AlterTable, AppendData, Assignment, CreateTableAsSelect, CreateTableStatement, CreateV2Table, DeleteAction, DeleteFromTable, DescribeRelation, DropTable, InsertAction, LocalRelation, LogicalPlan, MergeIntoTable, OneRowRelation, Project, SetTableLocation, SetTableProperties, ShowTableProperties, SubqueryAlias, UnsetTableProperties, UpdateAction, UpdateTable}
+import org.apache.spark.sql.catalyst.plans.logical.{AlterTable, AnalysisOnlyCommand, AppendData, Assignment, CreateTableAsSelect, CreateTableStatement, CreateV2Table, DeleteAction, DeleteFromTable, DescribeRelation, DropTable, InsertAction, LocalRelation, LogicalPlan, MergeIntoTable, OneRowRelation, Project, SetTableLocation, SetTableProperties, ShowTableProperties, SubqueryAlias, UnsetTableProperties, UpdateAction, UpdateTable}
 import org.apache.spark.sql.catalyst.rules.Rule
 import org.apache.spark.sql.connector.FakeV2Provider
 import org.apache.spark.sql.connector.catalog.{CatalogManager, CatalogNotFoundException, Identifier, Table, TableCapability, TableCatalog, TableChange, V1Table}
@@ -2192,6 +2192,15 @@ class PlanResolutionSuite extends AnalysisTest {
     assert(desc.comment == Some("no comment"))
   }
 
+  test("SPARK-34701: children/innerChildren should be mutually exclusive for AnalysisOnlyCommand") {
+    val cmdNotAnalyzed = DummyAnalysisOnlyCommand(isAnalyzed = false, childrenToAnalyze = Seq(null))
+    assert(cmdNotAnalyzed.innerChildren.isEmpty)
+    assert(cmdNotAnalyzed.children.length == 1)
+    val cmdAnalyzed = cmdNotAnalyzed.markAsAnalyzed()
+    assert(cmdAnalyzed.innerChildren.length == 1)
+    assert(cmdAnalyzed.children.isEmpty)
+  }
+
   // TODO: add tests for more commands.
 }
 
@@ -2201,3 +2210,13 @@ object AsDataSourceV2Relation {
     case _ => None
   }
 }
+
+case class DummyAnalysisOnlyCommand(
+    isAnalyzed: Boolean,
+    childrenToAnalyze: Seq[LogicalPlan]) extends AnalysisOnlyCommand {
+  override def markAsAnalyzed(): LogicalPlan = copy(isAnalyzed = true)
+  override protected def withNewChildrenInternal(
+      newChildren: IndexedSeq[LogicalPlan]): LogicalPlan = {
+    copy(childrenToAnalyze = newChildren)
+  }
+}