[GitHub] [spark] imback82 commented on a change in pull request #31273: [WIP][Spark-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

[GitBox <gi...@apache.org>]

imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r563194113



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -558,20 +560,27 @@ object ViewHelper {
       catalog: SessionCatalog, child: LogicalPlan): (Seq[Seq[String]], Seq[String]) = {
     def collectTempViews(child: LogicalPlan): Seq[Seq[String]] = {
       child.collect {
-        case UnresolvedRelation(nameParts, _, _) if catalog.isTempView(nameParts) =>
-          Seq(nameParts)
-        case plan if !plan.resolved => plan.expressions.flatMap(_.collect {
+        case s @ SubqueryAlias(_, view: View) if view.isTempView =>
+          Seq(s.identifier.qualifier :+ s.identifier.name)
+        case s: SubqueryAlias if s.getTagValue(SUBQUERY_TYPE_TAG).exists(_ == "tempView") =>

Review comment:
       I tried to wrap it with `View`, but encountered an issue where duplicate columns are not allowed for `CatalogTable.properties`: https://github.com/apache/spark/blob/0592503669c27bf3140160f665d71ae388d342c3/sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala#L461-L464
   
   Further looking, #30567 may have introduced a breaking change where the following works in Spark 3.0:
   ```
   Seq(1).map(i => (i, i)).toDF("i", "i").createTempView("tv1")
   sql("create temporary view tv2 as select 1 as i, 2 as i")
   ```
   , but in Spark 3.1
   ```
   Seq(1).map(i => (i, i)).toDF("i", "i").createTempView("tv1") // work
   sql("create temporary view tv2 as select 1 as i, 2 as i") // doesn't work!
   org.apache.spark.sql.AnalysisException: Found duplicate column(s) in the view definition: `i`
   ```
   
   Should I fix this first such that the behavior is consistent with DataFrame API / DDL (basically we can relax the check only for temporary views)? WDYT @cloud-fan / @viirya ?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org