You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/01/21 05:02:41 UTC

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

imback82 opened a new pull request #31273:
URL: https://github.com/apache/spark/pull/31273


   <!--
   Thanks for sending a pull request!  Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
     2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
     4. Be sure to keep the PR description updated to reflect all changes.
     5. Please write your PR title to summarize what this PR proposes.
     6. If possible, provide a concise example to reproduce the issue for a faster review.
     7. If you want to add a new configuration, please read the guideline first for naming configurations in
        'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
   -->
   
   ### What changes were proposed in this pull request?
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. 
   If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
     1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
     2. If you fix some SQL features, you can provide some references of other DBMSes.
     3. If there is design documentation, please add the link.
     4. If there is a discussion in the mailing list, please add the link.
   -->
   This PR proposes to make `CreateViewStatement.child` to be `LogicalPlan`'s `children` so that it's resolved in the analyze phase.
   
   ### Why are the changes needed?
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you propose a new API, clarify the use case for a new API.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   Currently, the `CreateViewStatement.child` is resolved when the create view command runs, which is inconsistent with other plan resolutions. For example, you may see the following in the physical plan:
   ```
   == Physical Plan ==
   Execute CreateViewCommand (1)
      +- CreateViewCommand (2)
            +- Project (4)
               +- UnresolvedRelation (3)
   ```
   
   ### Does this PR introduce _any_ user-facing change?
   <!--
   Note that it means *any* user-facing change including all aspects such as the documentation fix.
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
   If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
   If no, write 'No'.
   -->
   Yes. For the example, you will now see the resolved plan:
   ```
   == Physical Plan ==
   Execute CreateViewCommand (1)
      +- CreateViewCommand (2)
            +- Project (5)
               +- SubqueryAlias (4)
                  +- LogicalRelation (3)
   ```
   
   ### How was this patch tested?
   <!--
   If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
   If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
   If tests were not added, please describe why they were not added and/or why it was difficult to add.
   -->
   Updated existing tests.


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770562957


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134699/
   


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r581589456



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala
##########
@@ -443,21 +444,25 @@ case class InsertIntoDir(
 }
 
 /**
- * A container for holding the view description(CatalogTable), and the output of the view. The
- * child should be a logical plan parsed from the `CatalogTable.viewText`, should throw an error
- * if the `viewText` is not defined.
+ * A container for holding the view description(CatalogTable) and info whether the view is temporary
+ * or not. If the view description is available, the child should be a logical plan parsed from the

Review comment:
       `If the view description is available`, it's always available now. should be `if it's a SQL (temp) view, ...`




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784671202


   **[Test build #135392 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135392/testReport)** for PR 31273 at commit [`84d817c`](https://github.com/apache/spark/commit/84d817cdd11a33798bd1f4088b9e7e016b4ec74b).


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770311366


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134683/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770315633


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39270/
   


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

[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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r563908316



##########
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:
       Thanks @cloud-fan for the explanation.
   
   I think we have few ways to detect dataframe temp views:
   1. Add `isView: Boolean` field to the `SubqueryAlias`.
   2. If we want to wrap dataframe temp views with `View`, we need to create `CatalogTable` - which is `View.desc` - with relaxed requirements (e.g., allowing duplicate columns).
   3. Make `View.desc` optional and allow it to be `None` only for temporary views.
   
   I think 3) makes the most sense without changing semantics. WDYT?




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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778567456


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39716/
   


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

[GitHub] [spark] cloud-fan 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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r563926357



##########
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:
       Shall we not wrap dataframe temp view with `View`? The original goal is to make SQL temp view and permanent view consistent.




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

[GitHub] [spark] cloud-fan 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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r563957108



##########
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:
       OK I get your point now. If we need a new logical plan, then it's better to reuse `View` with some modification. +1 to make `View.desc` optional and set it to None for dataframe temp view. Or we can generate column names for dataframe temp view in `View.desc`, like `col1, col2, ...`, to bypass the check.




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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778667942


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39721/
   


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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-781715027






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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580815261



##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/ResolveHintsSuite.scala
##########
@@ -30,37 +30,48 @@ import org.apache.spark.sql.types.IntegerType
 class ResolveHintsSuite extends AnalysisTest {
   import org.apache.spark.sql.catalyst.analysis.TestRelations._
 
+  private def checkAnalysisWithTempViewEliminated(

Review comment:
       This appears twice. Shall we move it to `AnalysisTest`? And probably name it `checkAnalysisWithoutViewWrapper`




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778666233


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39721/
   


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580824030



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -541,40 +552,31 @@ object ViewHelper {
   }
 
   /**
-   * Collect all temporary views and functions and return the identifiers separately
-   * This func traverses the unresolved plan `child`. Below are the reasons:
-   * 1) Analyzer replaces unresolved temporary views by a SubqueryAlias with the corresponding
-   * logical plan. After replacement, it is impossible to detect whether the SubqueryAlias is
-   * added/generated from a temporary view.
-   * 2) The temp functions are represented by multiple classes. Most are inaccessible from this
-   * package (e.g., HiveGenericUDF).
+   * Collect all temporary views and functions and return the identifiers separately.
    */
   private def collectTemporaryObjects(
       catalog: SessionCatalog, child: LogicalPlan): (Seq[Seq[String]], Seq[String]) = {
     def collectTempViews(child: LogicalPlan): Seq[Seq[String]] = {
       child.flatMap {
-        case UnresolvedRelation(nameParts, _, _) if catalog.isTempView(nameParts) =>
-          Seq(nameParts)
-        case w: With if !w.resolved => w.innerChildren.flatMap(collectTempViews)
-        case plan if !plan.resolved => plan.expressions.flatMap(_.flatMap {
+        case s @ SubqueryAlias(_, view: View) if view.isTempView =>

Review comment:
       BTW does temp view come with `SubqueryAlias`? What's the qualifier there?




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-781667993


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39824/
   


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

[GitHub] [spark] SparkQA removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778530546


   **[Test build #135130 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135130/testReport)** for PR 31273 at commit [`fbdcf3b`](https://github.com/apache/spark/commit/fbdcf3b7535392ef0cd02d7ab32cbb3e5f8116b4).


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782474550


   **[Test build #135283 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135283/testReport)** for PR 31273 at commit [`e9cd2f3`](https://github.com/apache/spark/commit/e9cd2f332a56bb47446b10fde2fa0658dd834e7e).


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-786003048


   Thanks @LuciferYang, let me take a look.


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

[GitHub] [spark] SparkQA removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782474550


   **[Test build #135283 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135283/testReport)** for PR 31273 at commit [`e9cd2f3`](https://github.com/apache/spark/commit/e9cd2f332a56bb47446b10fde2fa0658dd834e7e).


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-769459247


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39217/
   


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

[GitHub] [spark] cloud-fan 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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r563510450



##########
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 think it's expected. The goal is to make SQL temp view and permanent view consistent, and forbidding duplicated column names is consistent.




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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r568167394



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/CacheManager.scala
##########
@@ -159,11 +159,19 @@ class CacheManager extends Logging with AdaptiveSparkPlanHelper {
       plan: LogicalPlan,
       cascade: Boolean,
       blocking: Boolean = false): Unit = {
+    val analyzedPlan = if (plan.resolved) {
+      plan
+    } else {
+      val qe = spark.sessionState.executePlan(plan)
+      qe.assertAnalyzed()

Review comment:
       ```
   sql("CACHE TABLE t1 AS SELECT * FROM testData")
   sql("UNCACHE TABLE t1")
   ```
   `t1` passed to `uncacheQuery` is:
   ```
   'SubqueryAlias t1
   +- View (`t1`, ['key,'value])
      +- 'Project [upcast('key, IntegerType) AS key#293, upcast('value, StringType) AS value#294]
         +- 'Project [*]
            +- 'UnresolvedRelation [testData], [], false
   ```
   Now that temporary views are always resolved before they are created, we need to resolve `t1` in this case.
   
   Note that we couldn't make the table of `CacheTable`/`UncacheTable` as a child due to https://github.com/apache/spark/pull/30598#discussion_r536384428.
   




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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770615880


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39293/
   


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r578363697



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -1101,9 +1116,9 @@ class Analyzer(override val catalogManager: CatalogManager)
       // The view's child should be a logical plan parsed from the `desc.viewText`, the variable
       // `viewText` should be defined, or else we throw an error on the generation of the View
       // operator.
-      case view @ View(desc, isTempView, child) if !child.resolved =>
+      case view @ View(Some(desc), isTempView, child) if !child.resolved =>
         // Resolve all the UnresolvedRelations and Views in the child.
-        val newChild = AnalysisContext.withAnalysisContext(desc) {
+        val newChild = AnalysisContext.withAnalysisContext(desc, isTempView) {

Review comment:
       I'm a bit confused here. For the new SQL temp view, we use `referredTempViewNames` to decide if we need to resolve temp views inside the view definition. What's wrong with 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.

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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770324995






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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770763435


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134707/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-765875254


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38982/
   


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

[GitHub] [spark] cloud-fan commented on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-783967099


   Let me take a closer look first. It's better to record the create time for df temp view correctly, if the effort is small.


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r581589456



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala
##########
@@ -443,21 +444,25 @@ case class InsertIntoDir(
 }
 
 /**
- * A container for holding the view description(CatalogTable), and the output of the view. The
- * child should be a logical plan parsed from the `CatalogTable.viewText`, should throw an error
- * if the `viewText` is not defined.
+ * A container for holding the view description(CatalogTable) and info whether the view is temporary
+ * or not. If the view description is available, the child should be a logical plan parsed from the

Review comment:
       `If the view description is available`, it's always available now. should be `if it's a dataframe temp view`




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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r561609651



##########
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:
       ~~Or is it safe to do: `case s: SubqueryAlias if catalog.isTempView(s.identifier.name)`?~~
   
   I see the comment "After replacement, it is impossible to detect whether the SubqueryAlias is added/generated from a temporary view." If we can add a field to `SubqueryAlias`, this should be easy to handle.




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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r579508292



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -840,14 +840,9 @@ class SessionCatalog(
     plan match {
       case viewInfo: TemporaryViewRelation =>
         fromCatalogTable(viewInfo.tableMeta, isTempView = true)
-      case v => v
-    }
-  }
-
-  def getTempViewSchema(plan: LogicalPlan): StructType = {

Review comment:
       This def was not being used, so I am removing 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.

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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r581497415



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -541,40 +552,31 @@ object ViewHelper {
   }
 
   /**
-   * Collect all temporary views and functions and return the identifiers separately
-   * This func traverses the unresolved plan `child`. Below are the reasons:
-   * 1) Analyzer replaces unresolved temporary views by a SubqueryAlias with the corresponding
-   * logical plan. After replacement, it is impossible to detect whether the SubqueryAlias is
-   * added/generated from a temporary view.
-   * 2) The temp functions are represented by multiple classes. Most are inaccessible from this
-   * package (e.g., HiveGenericUDF).
+   * Collect all temporary views and functions and return the identifiers separately.
    */
   private def collectTemporaryObjects(
       catalog: SessionCatalog, child: LogicalPlan): (Seq[Seq[String]], Seq[String]) = {
     def collectTempViews(child: LogicalPlan): Seq[Seq[String]] = {
       child.flatMap {
-        case UnresolvedRelation(nameParts, _, _) if catalog.isTempView(nameParts) =>
-          Seq(nameParts)
-        case w: With if !w.resolved => w.innerChildren.flatMap(collectTempViews)
-        case plan if !plan.resolved => plan.expressions.flatMap(_.flatMap {
+        case s @ SubqueryAlias(_, view: View) if view.isTempView =>

Review comment:
       > ah it's `child.flatMap`, seems we don't need to match `SubqueryAlias` here?
   
   Ah, yes. `SubqueryAlias` is not needed any more as `View` always has `CatalogTable`.
    
   > BTW does temp view come with `SubqueryAlias`? What's the qualifier there?
   
   Yes, temp views come with `SubqueryAlias` if used with `LookupTempView`:
   https://github.com/apache/spark/blob/7f27d33a3c538da6754a6c011b29aa7eb0dafe2c/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala#L884-L889
   
   The qualifier is empty for temp view, and global temp database name for global temp view.

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala
##########
@@ -443,21 +444,23 @@ case class InsertIntoDir(
 }
 
 /**
- * A container for holding the view description(CatalogTable), and the output of the view. The
- * child should be a logical plan parsed from the `CatalogTable.viewText`, should throw an error
- * if the `viewText` is not defined.
+ * A container for holding the view description(CatalogTable) and info whether the view is temporary
+ * or not. If the view description is available, the child should be a logical plan parsed from the
+ * `CatalogTable.viewText`. Otherwise, the view is a temporary one created from a dataframe; in this

Review comment:
       I updated with a bit more info; please let me know if I missed anything.




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782516808


   **[Test build #135283 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135283/testReport)** for PR 31273 at commit [`e9cd2f3`](https://github.com/apache/spark/commit/e9cd2f332a56bb47446b10fde2fa0658dd834e7e).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-783882301


   > > However, there were tests that compare analyzed plans which were failing due to CatalogTable.createTime difference
   > 
   > Can you elaborate on it? analyzed plan does not have table metadata.
   
   For example, tests like the following:
   https://github.com/apache/spark/blob/612d52315b8476dd588d75ce3001dee5786db747/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala#L3656
   When the temp view is looked up, `View` with `CatalogTable` is returned.


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

[GitHub] [spark] cloud-fan commented on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-783246992


   Looks pretty good. My last concern is changing the `View` signature to hold temp views, as it leads to many other changes. I agree it's a bit overkill to add another wrapper node for temp views, but can we use a special `CatalogTable` to indicate it's temp view? so that we don't need to change the `View` signature.


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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784821270


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39985/
   


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

[GitHub] [spark] SparkQA removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782423413


   **[Test build #135281 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135281/testReport)** for PR 31273 at commit [`d906e19`](https://github.com/apache/spark/commit/d906e1902b813acf4ceccbd1e87bcf33ee2002a4).


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

[GitHub] [spark] cloud-fan commented on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-783924839


   I see. Since previously we don't have the `View` wrapper for df temp view, I think no one really cares about the table metadata of a temp view. We can probably just set the `createTime` to 0 to avoid the test failures.


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-764403536


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134304/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784823052


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135392/
   


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r579006076



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
##########
@@ -143,6 +143,7 @@ abstract class Optimizer(catalogManager: CatalogManager)
       EliminateResolvedHint,
       EliminateSubqueryAliases,
       EliminateView,
+      EliminateDataFrameTempViews,

Review comment:
       Do we need to put this rule here? `EliminateView` should remove all `View` wrappers.




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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r567603929



##########
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:
       @cloud-fan / @viirya I updated to make `View.desc` optional to support dataframe temp views. Could you please check again? Thanks!




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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-764403536






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

[GitHub] [spark] SparkQA removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784798564


   **[Test build #135406 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135406/testReport)** for PR 31273 at commit [`82d58ba`](https://github.com/apache/spark/commit/82d58baa98235f912141b2b40cdcd3bf0bdbe744).


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-781706679


   **[Test build #135244 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135244/testReport)** for PR 31273 at commit [`bace91a`](https://github.com/apache/spark/commit/bace91a680e093ef4828495ee16a931769f2aca2).
    * This patch passes all tests.
    * This patch **does not merge cleanly**.
    * This patch adds no public classes.


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778544322


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39711/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-765887080


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38982/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-765881666


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134396/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-767170764


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39051/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778561691


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39716/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782557636


   **[Test build #135293 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135293/testReport)** for PR 31273 at commit [`9da3b9c`](https://github.com/apache/spark/commit/9da3b9cd6ff1b6080caf0cfa7ab0021b0a079879).


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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-781937243


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135254/
   


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r568439758



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/CacheManager.scala
##########
@@ -159,11 +159,19 @@ class CacheManager extends Logging with AdaptiveSparkPlanHelper {
       plan: LogicalPlan,
       cascade: Boolean,
       blocking: Boolean = false): Unit = {
+    val analyzedPlan = if (plan.resolved) {
+      plan
+    } else {
+      val qe = spark.sessionState.executePlan(plan)
+      qe.assertAnalyzed()

Review comment:
       `CacheTable` is tricky as the query can be complex and needs to go through the entire analyzer, `UncacheTable` should be fine as we can just update a few rules to handle it, which is similar to INSERT.




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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-764431528


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38891/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778539637


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39711/
   


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

[GitHub] [spark] cloud-fan commented on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784835942


   thanks, merging to master!


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r583065179



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -111,12 +111,11 @@ case class CreateViewCommand(
 
     // When creating a permanent view, not allowed to reference temporary objects.
     // This should be called after `qe.assertAnalyzed()` (i.e., `child` can be resolved)
-    verifyTemporaryObjectsNotExists(catalog, isTemporary, name, child)
+    verifyTemporaryObjectsNotExists(catalog, isTemporary, name, analyzedPlan)

Review comment:
       ~~@cloud-fan if we remove `sparkSession.sessionState.executePlan(child)` and use `child` directly, do you think there would be an issue similar to `CacheTable` since `child` will be an optimized plan, not an analyzed plan?~~




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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r581592786



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -111,12 +111,11 @@ case class CreateViewCommand(
 
     // When creating a permanent view, not allowed to reference temporary objects.
     // This should be called after `qe.assertAnalyzed()` (i.e., `child` can be resolved)
-    verifyTemporaryObjectsNotExists(catalog, isTemporary, name, child)
+    verifyTemporaryObjectsNotExists(catalog, isTemporary, name, analyzedPlan)

Review comment:
       since `child` is already resolved, we don't need to call `sparkSession.sessionState.executePlan(child)` and re-analyze it now.




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784700704


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39972/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770760593


   **[Test build #134707 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134707/testReport)** for PR 31273 at commit [`a0ba508`](https://github.com/apache/spark/commit/a0ba50895c5aafeb449505d952dd314a5ce7b594).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-781835341


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39834/
   


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r581583611



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -541,40 +552,31 @@ object ViewHelper {
   }
 
   /**
-   * Collect all temporary views and functions and return the identifiers separately
-   * This func traverses the unresolved plan `child`. Below are the reasons:
-   * 1) Analyzer replaces unresolved temporary views by a SubqueryAlias with the corresponding
-   * logical plan. After replacement, it is impossible to detect whether the SubqueryAlias is
-   * added/generated from a temporary view.
-   * 2) The temp functions are represented by multiple classes. Most are inaccessible from this
-   * package (e.g., HiveGenericUDF).
+   * Collect all temporary views and functions and return the identifiers separately.
    */
   private def collectTemporaryObjects(
       catalog: SessionCatalog, child: LogicalPlan): (Seq[Seq[String]], Seq[String]) = {
     def collectTempViews(child: LogicalPlan): Seq[Seq[String]] = {
       child.flatMap {
-        case UnresolvedRelation(nameParts, _, _) if catalog.isTempView(nameParts) =>
-          Seq(nameParts)
-        case w: With if !w.resolved => w.innerChildren.flatMap(collectTempViews)
-        case plan if !plan.resolved => plan.expressions.flatMap(_.flatMap {
+        case s @ SubqueryAlias(_, view: View) if view.isTempView =>

Review comment:
       Let's do it in followups.




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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784976433


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135406/
   


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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782437490






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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782560710


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39872/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-767177034


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134465/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778562198


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135130/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-765887080


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38982/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-767198376


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39051/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-769446499


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39217/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-764412483


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38891/
   


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r589876308



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -152,10 +151,11 @@ case class CreateViewCommand(
             sparkSession,
             analyzedPlan,
             aliasedPlan.schema,
-            originalText,
-            child))
+            originalText))
       } else {
-        aliasedPlan
+        TemporaryViewRelation(
+          prepareTemporaryViewFromDataFrame(name, aliasedPlan),

Review comment:
       Created https://github.com/apache/spark/pull/31783. Thanks!




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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r581623588



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -111,12 +111,11 @@ case class CreateViewCommand(
 
     // When creating a permanent view, not allowed to reference temporary objects.
     // This should be called after `qe.assertAnalyzed()` (i.e., `child` can be resolved)
-    verifyTemporaryObjectsNotExists(catalog, isTemporary, name, child)
+    verifyTemporaryObjectsNotExists(catalog, isTemporary, name, analyzedPlan)

Review comment:
       Just analyze it in `CacheTableAsSelectExec` and remove analyzing here?




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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r583065179



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -111,12 +111,11 @@ case class CreateViewCommand(
 
     // When creating a permanent view, not allowed to reference temporary objects.
     // This should be called after `qe.assertAnalyzed()` (i.e., `child` can be resolved)
-    verifyTemporaryObjectsNotExists(catalog, isTemporary, name, child)
+    verifyTemporaryObjectsNotExists(catalog, isTemporary, name, analyzedPlan)

Review comment:
       @cloud-fan if we remove `sparkSession.sessionState.executePlan(child)` and use `child` directly, do you think there would be an issue similar to `CacheTable` since `child` will be an optimized plan, not an analyzed plan? 




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784713500


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39972/
   


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r589515272



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -152,10 +151,11 @@ case class CreateViewCommand(
             sparkSession,
             analyzedPlan,
             aliasedPlan.schema,
-            originalText,
-            child))
+            originalText))
       } else {
-        aliasedPlan
+        TemporaryViewRelation(
+          prepareTemporaryViewFromDataFrame(name, aliasedPlan),

Review comment:
       good catch. will fix.




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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r589494855



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -152,10 +151,11 @@ case class CreateViewCommand(
             sparkSession,
             analyzedPlan,
             aliasedPlan.schema,
-            originalText,
-            child))
+            originalText))
       } else {
-        aliasedPlan
+        TemporaryViewRelation(
+          prepareTemporaryViewFromDataFrame(name, aliasedPlan),

Review comment:
       Here is a mistake: in the `if` branch, we call `prepareTemporaryView(viewIdent, ...`, and here we should also use `viewIdent` instead of `name`. Global temp view has 2-part name and the `name` may not be qualified (e.g. `df.createGlobalTempView`).
   
   @imback82 can you help to fix it with a test?




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784793000


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39985/
   


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r581630221



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -111,12 +111,11 @@ case class CreateViewCommand(
 
     // When creating a permanent view, not allowed to reference temporary objects.
     // This should be called after `qe.assertAnalyzed()` (i.e., `child` can be resolved)
-    verifyTemporaryObjectsNotExists(catalog, isTemporary, name, child)
+    verifyTemporaryObjectsNotExists(catalog, isTemporary, name, analyzedPlan)

Review comment:
       Ok let's fix it in followup then. We probably should also fix `CacheTableAsSelect` and resolve the `query` in the analyzer.




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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778468902


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39708/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784821826


   **[Test build #135392 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135392/testReport)** for PR 31273 at commit [`84d817c`](https://github.com/apache/spark/commit/84d817cdd11a33798bd1f4088b9e7e016b4ec74b).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r568438630



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/CacheManager.scala
##########
@@ -159,11 +159,19 @@ class CacheManager extends Logging with AdaptiveSparkPlanHelper {
       plan: LogicalPlan,
       cascade: Boolean,
       blocking: Boolean = false): Unit = {
+    val analyzedPlan = if (plan.resolved) {
+      plan
+    } else {
+      val qe = spark.sessionState.executePlan(plan)
+      qe.assertAnalyzed()

Review comment:
       Thanks for the reminds. We can't make the table the child of `CacheTable/UncacheTable`, but we can resolve the table in the analyzer like what we did for INSERT?




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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580107291



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -862,7 +862,17 @@ class Analyzer(override val catalogManager: CatalogManager)
   }
 
   private def isResolvingView: Boolean = AnalysisContext.get.catalogAndNamespace.nonEmpty
-  private def referredTempViewNames: Seq[Seq[String]] = AnalysisContext.get.referredTempViewNames
+  private def isReferredTempViewName(nameParts: Seq[String]): Boolean = {
+    AnalysisContext.get.referredTempViewNames.exists { n =>
+      (n.length == nameParts.length) && n.zip(nameParts).forall {
+        case (a, b) => resolver(a, b)
+      }
+    }
+  }
+
+  private def getTempViewRawPlan(plan: LogicalPlan): LogicalPlan = {
+    EliminateDataFrameTempViews(EliminateSubqueryAliases(plan))

Review comment:
       `EliminateDataFrameTempViews` doesn't need to be a rule anymore, right? we can probably just do 
   ```
   EliminateSubqueryAliases(plan) match {
     case v: View if v.isDataFrameTempView => v.child
     case other => other
   }
   ```




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

[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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r563908316



##########
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:
       Thanks @cloud-fan for the explanation.
   
   I think we have few ways to detect dataframe temp views:
   1. Add `isView: Boolean` field to the `SubqueryAlias`.
   2. If we want to wrap dataframe temp views with `View`, we need to create `CatalogTable` - which is `View.desc` - with relaxed requirements (e.g., allowing duplicate columns).
   3. Make `View.desc` optional and allow it to be `None` only for temporary views.
   
   I think 3) makes the most sense without changing semantics. 




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-781822794


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39834/
   


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

[GitHub] [spark] SparkQA removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782437553


   **[Test build #135282 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135282/testReport)** for PR 31273 at commit [`4e55a1f`](https://github.com/apache/spark/commit/4e55a1f4fac4720782670aafe3e2fa3f9252ec6b).


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r568438630



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/CacheManager.scala
##########
@@ -159,11 +159,19 @@ class CacheManager extends Logging with AdaptiveSparkPlanHelper {
       plan: LogicalPlan,
       cascade: Boolean,
       blocking: Boolean = false): Unit = {
+    val analyzedPlan = if (plan.resolved) {
+      plan
+    } else {
+      val qe = spark.sessionState.executePlan(plan)
+      qe.assertAnalyzed()

Review comment:
       Thanks for the reminds. We can't make the table the child of `CacheTable/UncacheTable`, but we can resolve the table in the analyzer like what we did for INSERT?

##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/CacheManager.scala
##########
@@ -159,11 +159,19 @@ class CacheManager extends Logging with AdaptiveSparkPlanHelper {
       plan: LogicalPlan,
       cascade: Boolean,
       blocking: Boolean = false): Unit = {
+    val analyzedPlan = if (plan.resolved) {
+      plan
+    } else {
+      val qe = spark.sessionState.executePlan(plan)
+      qe.assertAnalyzed()

Review comment:
       `CacheTable` is tricky as the query can be complex and needs to go through the entire analyzer, `UncacheTable` should be fine as we can just update a few rules to handle it, which is similar to INSERT.




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

[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

Posted by 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`
   ```
   
   Is this a bug that I should fix separately 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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r561609651



##########
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:
       Or is it safe to do: `case s: SubqueryAlias if catalog.isTempView(s.identifier.name)`?
   
   I see the comment "After replacement, it is impossible to detect whether the SubqueryAlias is added/generated from a temporary view." If we can add a field to `SubqueryAlias`, this should be easy to handle.




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-767158619


   **[Test build #134465 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134465/testReport)** for PR 31273 at commit [`4b3f184`](https://github.com/apache/spark/commit/4b3f184ec49b14a6c445ea57d6f562564ef3474d).


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-764431528


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38891/
   


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

[GitHub] [spark] SparkQA removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778660542


   **[Test build #135140 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135140/testReport)** for PR 31273 at commit [`e92cbe1`](https://github.com/apache/spark/commit/e92cbe1427dacb8f6b1f7e967c4d1c6a75c32ebf).


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580738060



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -541,40 +539,33 @@ object ViewHelper {
   }
 
   /**
-   * Collect all temporary views and functions and return the identifiers separately
-   * This func traverses the unresolved plan `child`. Below are the reasons:
-   * 1) Analyzer replaces unresolved temporary views by a SubqueryAlias with the corresponding
-   * logical plan. After replacement, it is impossible to detect whether the SubqueryAlias is
-   * added/generated from a temporary view.
-   * 2) The temp functions are represented by multiple classes. Most are inaccessible from this
-   * package (e.g., HiveGenericUDF).
+   * Collect all temporary views and functions and return the identifiers separately.
    */
   private def collectTemporaryObjects(
       catalog: SessionCatalog, child: LogicalPlan): (Seq[Seq[String]], Seq[String]) = {
     def collectTempViews(child: LogicalPlan): Seq[Seq[String]] = {
       child.flatMap {
-        case UnresolvedRelation(nameParts, _, _) if catalog.isTempView(nameParts) =>
-          Seq(nameParts)
-        case w: With if !w.resolved => w.innerChildren.flatMap(collectTempViews)
-        case plan if !plan.resolved => plan.expressions.flatMap(_.flatMap {
+        case s @ SubqueryAlias(_, view: View) if view.isTempView =>
+          Seq(s.identifier.qualifier :+ s.identifier.name)
+        case w: With => w.innerChildren.flatMap(collectTempViews)

Review comment:
       👍🏼 




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-765870075


   **[Test build #134396 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134396/testReport)** for PR 31273 at commit [`8dc1961`](https://github.com/apache/spark/commit/8dc19613500b6301d7c666680fbb153ca41fb04b).


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r561620650



##########
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:
       This is a good point. We probably should use `View` to wrap temp view as well, to be truly consistent with permanent views. cc @linhongliu-db 




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-764357237






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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778667942


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39721/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770367042


   **[Test build #134688 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134688/testReport)** for PR 31273 at commit [`4e77301`](https://github.com/apache/spark/commit/4e77301c382807f9f2c775f9e59e0798b0dd0548).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds the following public classes _(experimental)_:
     * `trait UserDefinedExpression `


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

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

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-785866293


   `Exception when attempting to uncache xxx` similar logs have been printed more than 3700 times in [test log](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135392/consoleText)


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782442019


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135282/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-764357237


   **[Test build #134304 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134304/testReport)** for PR 31273 at commit [`71c01e8`](https://github.com/apache/spark/commit/71c01e85c7eb9a530607078730dd5cb8bc533be6).


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-767177034


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134465/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770348002


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39274/
   


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r575511343



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/CacheManager.scala
##########
@@ -159,11 +159,19 @@ class CacheManager extends Logging with AdaptiveSparkPlanHelper {
       plan: LogicalPlan,
       cascade: Boolean,
       blocking: Boolean = false): Unit = {
+    val analyzedPlan = if (plan.resolved) {
+      plan
+    } else {
+      val qe = spark.sessionState.executePlan(plan)
+      qe.assertAnalyzed()

Review comment:
       Actually, it was subtle why `UnresolvedRelation [testData]` inside temp view `t1` was not resolved.
   When the temp view `t1` is looked up, its text is parsed and produces:
   ```
   'SubqueryAlias t1
   +- View (`t1`, ['key,'value])
      +- 'Project [upcast('key, IntegerType) AS key#293, upcast('value, StringType) AS value#294]
         +- 'Project [*]
            +- 'UnresolvedRelation [testData], [], false
   ```
   Now this logical plan goes thru the following to resolve this view further: https://github.com/apache/spark/blob/e0053853c90d39ef6de9d59fb933525e20bae1fa/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala#L1100
   
   However, we have the following check that doesn't allow resolving further:
   https://github.com/apache/spark/blob/e0053853c90d39ef6de9d59fb933525e20bae1fa/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala#L924-L925
   
   But, we are resolving a temp view in this context, so I updated the check to allow relations in temp views are resolved correctly.




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-781918507


   **[Test build #135254 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135254/testReport)** for PR 31273 at commit [`9432fec`](https://github.com/apache/spark/commit/9432fec05a7c2ca1e83c6811d12db343e9e56015).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds the following public classes _(experimental)_:
     * `case class AddPartitionExec(`
     * `case class DropPartitionExec(`
     * `case class RenamePartitionExec(`


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r581501103



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -541,40 +552,31 @@ object ViewHelper {
   }
 
   /**
-   * Collect all temporary views and functions and return the identifiers separately
-   * This func traverses the unresolved plan `child`. Below are the reasons:
-   * 1) Analyzer replaces unresolved temporary views by a SubqueryAlias with the corresponding
-   * logical plan. After replacement, it is impossible to detect whether the SubqueryAlias is
-   * added/generated from a temporary view.
-   * 2) The temp functions are represented by multiple classes. Most are inaccessible from this
-   * package (e.g., HiveGenericUDF).
+   * Collect all temporary views and functions and return the identifiers separately.
    */
   private def collectTemporaryObjects(
       catalog: SessionCatalog, child: LogicalPlan): (Seq[Seq[String]], Seq[String]) = {
     def collectTempViews(child: LogicalPlan): Seq[Seq[String]] = {
       child.flatMap {
-        case UnresolvedRelation(nameParts, _, _) if catalog.isTempView(nameParts) =>
-          Seq(nameParts)
-        case w: With if !w.resolved => w.innerChildren.flatMap(collectTempViews)
-        case plan if !plan.resolved => plan.expressions.flatMap(_.flatMap {
+        case s @ SubqueryAlias(_, view: View) if view.isTempView =>

Review comment:
       BTW, this is not complete until we migrate `ALTER VIEW AS` and `CREATE TEMP VIEW USING` since they may store analyzed plan directly. Do you want to me update these commands in this PR?




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778558253


   **[Test build #135135 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135135/testReport)** for PR 31273 at commit [`2a3e5b7`](https://github.com/apache/spark/commit/2a3e5b74cfa5277fd3b72db38b205901474294e6).


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r575621089



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -940,7 +949,15 @@ class Analyzer(override val catalogManager: CatalogManager)
   // If we are resolving relations insides views, we need to expand single-part relation names with
   // the current catalog and namespace of when the view was created.
   private def expandRelationName(nameParts: Seq[String]): Seq[String] = {
-    if (!isResolvingView || referredTempViewNames.contains(nameParts)) return nameParts
+    def isReferredTempViewName(nameParts: Seq[String]): Boolean = {
+      referredTempViewNames.exists { n =>
+        (n.length == nameParts.length) && n.zip(nameParts).forall {
+          case (a, b) => conf.resolver(a, b)

Review comment:
       ~~Not handling case sensitivity seems to be an existing bug.~~




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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r581592514



##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisTest.scala
##########
@@ -59,6 +59,28 @@ trait AnalysisTest extends PlanTest {
     }
   }
 
+  protected def checkAnalysisWithTransform(
+      inputPlan: LogicalPlan,
+      expectedPlan: LogicalPlan,
+      caseSensitive: Boolean = true)(transform: LogicalPlan => LogicalPlan): Unit = {
+    withSQLConf(SQLConf.CASE_SENSITIVE.key -> caseSensitive.toString) {
+      val analyzer = getAnalyzer
+      val actualPlan = analyzer.executeAndCheck(inputPlan, new QueryPlanningTracker)
+      comparePlans(transform(actualPlan), expectedPlan)
+    }
+  }
+
+  protected def checkAnalysisWithoutViewWrapper(
+      inputPlan: LogicalPlan,
+      expectedPlan: LogicalPlan,
+      caseSensitive: Boolean = true): Unit = {
+    checkAnalysisWithTransform(inputPlan, expectedPlan, caseSensitive) { plan =>

Review comment:
       shall we inline `checkAnalysisWithTransform` here?




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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580108792



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -622,8 +622,7 @@ class SessionCatalog(
   }
 
   /**
-   * Generate a [[View]] operator from the view description if the view stores sql text,
-   * otherwise, it is same to `getRawTempView`
+   * Generate a [[View]] operator from the temporary view stored.

Review comment:
       let's update `getGlobalTempView` as well.




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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-767177034






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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-765881640


   **[Test build #134396 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134396/testReport)** for PR 31273 at commit [`8dc1961`](https://github.com/apache/spark/commit/8dc19613500b6301d7c666680fbb153ca41fb04b).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


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

[GitHub] [spark] SparkQA removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784671202


   **[Test build #135392 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135392/testReport)** for PR 31273 at commit [`84d817c`](https://github.com/apache/spark/commit/84d817cdd11a33798bd1f4088b9e7e016b4ec74b).


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r561604269



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala
##########
@@ -2029,7 +2029,7 @@ class DataSourceV2SQLSuite
   test("CREATE VIEW") {
     val v = "testcat.ns1.ns2.v"
     val e = intercept[AnalysisException] {
-      sql(s"CREATE VIEW $v AS SELECT * FROM tab1")
+      sql(s"CREATE VIEW $v AS SELECT 1")

Review comment:
       This needs to be updated now that `ResolveSessionCatalog` handles `CreateViewStatement` only if its `child` is resolved.

##########
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:
       This seems a bit hacky, but `SubqueryAlias(_, view: View)` couldn't handle all the cases. For example,
   ```scala
   spark.range(10).createTempView("t")
   sql("CREATE VIEW v AS SELECT * FROM t")
   ```
   The `child` is:
   ```
   Project [id#16L]
   +- SubqueryAlias t
      +- Range (0, 10, step=1, splits=Some(2))
   ```
   @cloud-fan, @viirya Any suggestion to capture the view properly? Does it make sense to add an `isView` field to `SubqueryAlias`?

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -1086,7 +1086,7 @@ class Analyzer(override val catalogManager: CatalogManager)
         }
         view.copy(child = newChild)
       case p @ SubqueryAlias(_, view: View) =>
-        p.copy(child = resolveViews(view))
+        p.makeCopy(Array(p.identifier, resolveViews(view)))

Review comment:
       This is to copy the tags, but seems hacky since `copy` can be called to remove the tags in other places. Please check https://github.com/apache/spark/pull/31273#discussion_r561607183 if it makes sense to add `isView` to `SubqueryAlias`.

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -1086,7 +1086,7 @@ class Analyzer(override val catalogManager: CatalogManager)
         }
         view.copy(child = newChild)
       case p @ SubqueryAlias(_, view: View) =>
-        p.copy(child = resolveViews(view))
+        p.makeCopy(Array(p.identifier, resolveViews(view)))

Review comment:
       This is to copy the tags, but seems hacky since `copy` can be called in other places, which will remove the tags. Please check https://github.com/apache/spark/pull/31273#discussion_r561607183 if it makes sense to add `isView` to `SubqueryAlias`.

##########
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:
       Or is it safe to do: `case s: SubqueryAlias if catalog.isTempView(s.identifier.name)`?

##########
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:
       Or is it safe to do: `case s: SubqueryAlias if catalog.isTempView(s.identifier.name)`?
   
   I see the comment "After replacement, it is impossible to detect whether the SubqueryAlias is added/generated from a temporary view." If we can add a field to `SubqueryAlias`, this should be easy to handle.

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -1363,9 +1368,14 @@ class SessionCatalog(
       Utils.classForName("org.apache.spark.sql.expressions.UserDefinedAggregateFunction")
     if (clsForUDAF.isAssignableFrom(clazz)) {
       val cls = Utils.classForName("org.apache.spark.sql.execution.aggregate.ScalaUDAF")
-      val e = cls.getConstructor(classOf[Seq[Expression]], clsForUDAF, classOf[Int], classOf[Int])
-        .newInstance(input,
-          clazz.getConstructor().newInstance().asInstanceOf[Object], Int.box(1), Int.box(1))
+      val e = cls.getConstructor(

Review comment:
       Yea, we need to collect temp functions by name. Currently, UDAF doesn't store a name, so I added it.

##########
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:
       ~~Or is it safe to do: `case s: SubqueryAlias if catalog.isTempView(s.identifier.name)`?~~
   
   I see the comment "After replacement, it is impossible to detect whether the SubqueryAlias is added/generated from a temporary view." If we can add a field to `SubqueryAlias`, this should be easy to handle.

##########
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:
       Thanks, will try that.




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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r575621089



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -940,7 +949,15 @@ class Analyzer(override val catalogManager: CatalogManager)
   // If we are resolving relations insides views, we need to expand single-part relation names with
   // the current catalog and namespace of when the view was created.
   private def expandRelationName(nameParts: Seq[String]): Seq[String] = {
-    if (!isResolvingView || referredTempViewNames.contains(nameParts)) return nameParts
+    def isReferredTempViewName(nameParts: Seq[String]): Boolean = {
+      referredTempViewNames.exists { n =>
+        (n.length == nameParts.length) && n.zip(nameParts).forall {
+          case (a, b) => conf.resolver(a, b)

Review comment:
       Not handling case sensitivity seems to be an existing bug.




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770311319


   **[Test build #134683 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134683/testReport)** for PR 31273 at commit [`ac663aa`](https://github.com/apache/spark/commit/ac663aa0dc50f129ab0f65d2723d76c6a3e97b3c).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r579010493



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -152,10 +152,10 @@ case class CreateViewCommand(
             sparkSession,
             analyzedPlan,
             aliasedPlan.schema,
-            originalText,
-            child))
+            originalText))
       } else {
-        aliasedPlan
+        assert(isTemporary)
+        View(None, isTemporary, aliasedPlan)

Review comment:
       It's a bit weird to create the `View` wrapper at view creation time. Shall we do it in `SessionCatalog.lookupTempView`?




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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r561620650



##########
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:
       This is a good point. We probably should use `View` to wrap temp view as well, to be truly consistent with permanent views. cc @linhongliu-db 




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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580817665



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -541,40 +552,31 @@ object ViewHelper {
   }
 
   /**
-   * Collect all temporary views and functions and return the identifiers separately
-   * This func traverses the unresolved plan `child`. Below are the reasons:
-   * 1) Analyzer replaces unresolved temporary views by a SubqueryAlias with the corresponding
-   * logical plan. After replacement, it is impossible to detect whether the SubqueryAlias is
-   * added/generated from a temporary view.
-   * 2) The temp functions are represented by multiple classes. Most are inaccessible from this
-   * package (e.g., HiveGenericUDF).
+   * Collect all temporary views and functions and return the identifiers separately.
    */
   private def collectTemporaryObjects(
       catalog: SessionCatalog, child: LogicalPlan): (Seq[Seq[String]], Seq[String]) = {
     def collectTempViews(child: LogicalPlan): Seq[Seq[String]] = {
       child.flatMap {
-        case UnresolvedRelation(nameParts, _, _) if catalog.isTempView(nameParts) =>
-          Seq(nameParts)
-        case w: With if !w.resolved => w.innerChildren.flatMap(collectTempViews)
-        case plan if !plan.resolved => plan.expressions.flatMap(_.flatMap {
+        case s @ SubqueryAlias(_, view: View) if view.isTempView =>

Review comment:
       is it an existing but that we missed handling `SubqueryAlias`?

##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -541,40 +552,31 @@ object ViewHelper {
   }
 
   /**
-   * Collect all temporary views and functions and return the identifiers separately
-   * This func traverses the unresolved plan `child`. Below are the reasons:
-   * 1) Analyzer replaces unresolved temporary views by a SubqueryAlias with the corresponding
-   * logical plan. After replacement, it is impossible to detect whether the SubqueryAlias is
-   * added/generated from a temporary view.
-   * 2) The temp functions are represented by multiple classes. Most are inaccessible from this
-   * package (e.g., HiveGenericUDF).
+   * Collect all temporary views and functions and return the identifiers separately.
    */
   private def collectTemporaryObjects(
       catalog: SessionCatalog, child: LogicalPlan): (Seq[Seq[String]], Seq[String]) = {
     def collectTempViews(child: LogicalPlan): Seq[Seq[String]] = {
       child.flatMap {
-        case UnresolvedRelation(nameParts, _, _) if catalog.isTempView(nameParts) =>
-          Seq(nameParts)
-        case w: With if !w.resolved => w.innerChildren.flatMap(collectTempViews)
-        case plan if !plan.resolved => plan.expressions.flatMap(_.flatMap {
+        case s @ SubqueryAlias(_, view: View) if view.isTempView =>

Review comment:
       is it an existing bug that we missed handling `SubqueryAlias`?




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784798564


   **[Test build #135406 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135406/testReport)** for PR 31273 at commit [`82d58ba`](https://github.com/apache/spark/commit/82d58baa98235f912141b2b40cdcd3bf0bdbe744).


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-769468471


   **[Test build #134629 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134629/testReport)** for PR 31273 at commit [`bfabe9f`](https://github.com/apache/spark/commit/bfabe9f60e472d058aca3fc9837431c960f9380c).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r579002147



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -923,9 +933,9 @@ class Analyzer(override val catalogManager: CatalogManager)
     def lookupTempView(
         identifier: Seq[String], isStreaming: Boolean = false): Option[LogicalPlan] = {
       // Permanent View can't refer to temp views, no need to lookup at all.
-      if (isResolvingView && !referredTempViewNames.contains(identifier)) return None
+      if (isResolvingView && !isReferredTempViewName(identifier)) return None
 
-      val tmpView = identifier match {
+      val tmpView = expandRelationName(identifier) match {

Review comment:
       what does this fix?




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-764400591


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38891/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770369373


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134688/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-764357237


   **[Test build #134304 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134304/testReport)** for PR 31273 at commit [`71c01e8`](https://github.com/apache/spark/commit/71c01e85c7eb9a530607078730dd5cb8bc533be6).


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770540322


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39285/
   


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r579004852



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -1129,9 +1139,12 @@ class Analyzer(override val catalogManager: CatalogManager)
           case other => other
         }
 
-        EliminateSubqueryAliases(relation) match {
-          case v: View =>
-            throw QueryCompilationErrors.insertIntoViewNotAllowedError(v.desc.identifier, table)
+        // Inserting into a file-based temporary view is allowed.
+        // (e.g., spark.read.parquet("path").createOrReplaceTempView("t").
+        // Thus, apply EliminateDataFrameTempViews rule to remove any dataframe temp views.
+        EliminateDataFrameTempViews(EliminateSubqueryAliases(relation)) match {

Review comment:
       we can probably add a `def getTempViewRawPlan` to avoid duplicated code.




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782428002


   **[Test build #135281 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135281/testReport)** for PR 31273 at commit [`d906e19`](https://github.com/apache/spark/commit/d906e1902b813acf4ceccbd1e87bcf33ee2002a4).
    * This patch **fails to build**.
    * This patch merges cleanly.
    * This patch adds the following public classes _(experimental)_:
     * `case class StringTrimBoth(srcStr: Expression, trimStr: Option[Expression], child: Expression)`
     * `  case class TypedPartValue(value: String, dataType: DataType)`
     * `  case class PartitionValues(columnNames: Seq[String], typedValues: Seq[TypedPartValue])`


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782564125


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39872/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770615880


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39293/
   


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

[GitHub] [spark] cloud-fan commented on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-786003958


   @LuciferYang thanks for reporting! I'm fixing it in https://github.com/apache/spark/pull/31650


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580111666



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -111,7 +111,7 @@ case class CreateViewCommand(
 
     // When creating a permanent view, not allowed to reference temporary objects.
     // This should be called after `qe.assertAnalyzed()` (i.e., `child` can be resolved)
-    verifyTemporaryObjectsNotExists(catalog, isTemporary, name, child)
+    verifyTemporaryObjectsNotExists(catalog, isTemporary, name, analyzedPlan)

Review comment:
       can we remove `L98` to `L101`? It seems we don't need to analyze the plan inside `run` method again.




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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782442019






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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580816822



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -192,6 +192,17 @@ case class CreateViewCommand(
     Seq.empty[Row]
   }
 
+  /**
+   * Checks if the temp view (the result of getTempViewRawPlan or getRawGlobalTempView) is storing
+   * the same plan as the given aliased plan.
+   */
+  private def isSamePlan(
+      rawTempView: Option[LogicalPlan],
+      aliasedPlan: LogicalPlan): Boolean = rawTempView match {
+    case Some(TemporaryViewRelation(_, Some(p))) => p.sameResult(aliasedPlan)

Review comment:
       It's possible that `rawTempView` is not `TemporaryViewRelation`, shall we add
   ```
   ...
   case Some(p) => p.sameResult(aliasedPlan)
   ```




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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r561609651



##########
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:
       Or is it safe to do: `case s: SubqueryAlias if catalog.isTempView(s.identifier.name)`?




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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782592079


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135293/
   


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r568167843



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/udaf.scala
##########
@@ -325,13 +325,16 @@ case class ScalaUDAF(
     children: Seq[Expression],
     udaf: UserDefinedAggregateFunction,
     mutableAggBufferOffset: Int = 0,
-    inputAggBufferOffset: Int = 0)
+    inputAggBufferOffset: Int = 0,
+    udafName: Option[String] = None)

Review comment:
       Ok, I will address this as a separate PR first.




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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r567840441



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/udaf.scala
##########
@@ -325,13 +325,16 @@ case class ScalaUDAF(
     children: Seq[Expression],
     udaf: UserDefinedAggregateFunction,
     mutableAggBufferOffset: Int = 0,
-    inputAggBufferOffset: Int = 0)
+    inputAggBufferOffset: Int = 0,
+    udafName: Option[String] = None)

Review comment:
       I think this worths a separate PR: adding a name to UDF for better EXPLAIN result.




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770315763


   **[Test build #134685 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134685/testReport)** for PR 31273 at commit [`0fbf1cf`](https://github.com/apache/spark/commit/0fbf1cf58032e923e468c5c51c165535d8d7e690).


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-769469355






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

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

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-769415691


   **[Test build #134629 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134629/testReport)** for PR 31273 at commit [`bfabe9f`](https://github.com/apache/spark/commit/bfabe9f60e472d058aca3fc9837431c960f9380c).


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770609908


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39293/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770594667


   **[Test build #134707 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134707/testReport)** for PR 31273 at commit [`a0ba508`](https://github.com/apache/spark/commit/a0ba50895c5aafeb449505d952dd314a5ce7b594).


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770337439


   **[Test build #134688 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134688/testReport)** for PR 31273 at commit [`4e77301`](https://github.com/apache/spark/commit/4e77301c382807f9f2c775f9e59e0798b0dd0548).


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-781709674


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39824/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-765870075


   **[Test build #134396 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134396/testReport)** for PR 31273 at commit [`8dc1961`](https://github.com/apache/spark/commit/8dc19613500b6301d7c666680fbb153ca41fb04b).


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782423413


   **[Test build #135281 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135281/testReport)** for PR 31273 at commit [`d906e19`](https://github.com/apache/spark/commit/d906e1902b813acf4ceccbd1e87bcf33ee2002a4).


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580110717



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -541,40 +539,33 @@ object ViewHelper {
   }
 
   /**
-   * Collect all temporary views and functions and return the identifiers separately
-   * This func traverses the unresolved plan `child`. Below are the reasons:
-   * 1) Analyzer replaces unresolved temporary views by a SubqueryAlias with the corresponding
-   * logical plan. After replacement, it is impossible to detect whether the SubqueryAlias is
-   * added/generated from a temporary view.
-   * 2) The temp functions are represented by multiple classes. Most are inaccessible from this
-   * package (e.g., HiveGenericUDF).
+   * Collect all temporary views and functions and return the identifiers separately.
    */
   private def collectTemporaryObjects(
       catalog: SessionCatalog, child: LogicalPlan): (Seq[Seq[String]], Seq[String]) = {
     def collectTempViews(child: LogicalPlan): Seq[Seq[String]] = {
       child.flatMap {
-        case UnresolvedRelation(nameParts, _, _) if catalog.isTempView(nameParts) =>
-          Seq(nameParts)
-        case w: With if !w.resolved => w.innerChildren.flatMap(collectTempViews)
-        case plan if !plan.resolved => plan.expressions.flatMap(_.flatMap {
+        case s @ SubqueryAlias(_, view: View) if view.isTempView =>
+          Seq(s.identifier.qualifier :+ s.identifier.name)
+        case w: With => w.innerChildren.flatMap(collectTempViews)
+        case plan => plan.expressions.flatMap(_.flatMap {
           case e: SubqueryExpression => collectTempViews(e.plan)
           case _ => Seq.empty
         })
-        case _ => Seq.empty
       }.distinct
     }
 
     def collectTempFunctions(child: LogicalPlan): Seq[String] = {
       child.flatMap {
-        case w: With if !w.resolved => w.innerChildren.flatMap(collectTempFunctions)
-        case plan if !plan.resolved =>
+        case w: With => w.innerChildren.flatMap(collectTempFunctions)

Review comment:
       ditto




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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580106368



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -862,7 +862,13 @@ class Analyzer(override val catalogManager: CatalogManager)
   }
 
   private def isResolvingView: Boolean = AnalysisContext.get.catalogAndNamespace.nonEmpty
-  private def referredTempViewNames: Seq[Seq[String]] = AnalysisContext.get.referredTempViewNames
+  private def isReferredTempViewName(nameParts: Seq[String]): Boolean = {
+    AnalysisContext.get.referredTempViewNames.exists { n =>
+      (n.length == nameParts.length) && n.zip(nameParts).forall {

Review comment:
       ok let's leave it for now.




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-764398499


   **[Test build #134304 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134304/testReport)** for PR 31273 at commit [`71c01e8`](https://github.com/apache/spark/commit/71c01e85c7eb9a530607078730dd5cb8bc533be6).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r581587329



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -1139,7 +1152,10 @@ class Analyzer(override val catalogManager: CatalogManager)
           case other => other
         }
 
-        EliminateSubqueryAliases(relation) match {
+        // Inserting into a file-based temporary view is allowed.
+        // (e.g., spark.read.parquet("path").createOrReplaceTempView("t").
+        // Thus, we need to look at the raw plan of a temporary view.
+        getTempViewRawPlan(relation) match {

Review comment:
       Ah sorry I missed this. The relation here can be a table relation, not temp view, so `unwrapRelationPlan` maybe a better name.
   
   We can fix it in followup if there are no other comments to address.




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

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

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r561610487



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -1363,9 +1368,14 @@ class SessionCatalog(
       Utils.classForName("org.apache.spark.sql.expressions.UserDefinedAggregateFunction")
     if (clsForUDAF.isAssignableFrom(clazz)) {
       val cls = Utils.classForName("org.apache.spark.sql.execution.aggregate.ScalaUDAF")
-      val e = cls.getConstructor(classOf[Seq[Expression]], clsForUDAF, classOf[Int], classOf[Int])
-        .newInstance(input,
-          clazz.getConstructor().newInstance().asInstanceOf[Object], Int.box(1), Int.box(1))
+      val e = cls.getConstructor(

Review comment:
       There are some UDAF related change, is it related to the view stuff here?




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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r561607878



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -1086,7 +1086,7 @@ class Analyzer(override val catalogManager: CatalogManager)
         }
         view.copy(child = newChild)
       case p @ SubqueryAlias(_, view: View) =>
-        p.copy(child = resolveViews(view))
+        p.makeCopy(Array(p.identifier, resolveViews(view)))

Review comment:
       This is to copy the tags, but seems hacky since `copy` can be called in other places, which will remove the tags. Please check https://github.com/apache/spark/pull/31273#discussion_r561607183 if it makes sense to add `isView` to `SubqueryAlias`.




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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-767198376


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39051/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770311366






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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778530546


   **[Test build #135130 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135130/testReport)** for PR 31273 at commit [`fbdcf3b`](https://github.com/apache/spark/commit/fbdcf3b7535392ef0cd02d7ab32cbb3e5f8116b4).


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r578996797



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -862,7 +862,13 @@ class Analyzer(override val catalogManager: CatalogManager)
   }
 
   private def isResolvingView: Boolean = AnalysisContext.get.catalogAndNamespace.nonEmpty
-  private def referredTempViewNames: Seq[Seq[String]] = AnalysisContext.get.referredTempViewNames
+  private def isReferredTempViewName(nameParts: Seq[String]): Boolean = {
+    AnalysisContext.get.referredTempViewNames.exists { n =>
+      (n.length == nameParts.length) && n.zip(nameParts).forall {

Review comment:
       nit: `...forall(conf.resolver)`




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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778549438


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39711/
   


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r561621537



##########
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:
       Thanks, will try that.




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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-781848158


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39834/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770315763


   **[Test build #134685 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134685/testReport)** for PR 31273 at commit [`0fbf1cf`](https://github.com/apache/spark/commit/0fbf1cf58032e923e468c5c51c165535d8d7e690).


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-767158619






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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778530482


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135127/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778584531


   **[Test build #135135 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135135/testReport)** for PR 31273 at commit [`2a3e5b7`](https://github.com/apache/spark/commit/2a3e5b74cfa5277fd3b72db38b205901474294e6).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770305305


   **[Test build #134683 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134683/testReport)** for PR 31273 at commit [`ac663aa`](https://github.com/apache/spark/commit/ac663aa0dc50f129ab0f65d2723d76c6a3e97b3c).


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

[GitHub] [spark] SparkQA removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-781801473


   **[Test build #135254 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135254/testReport)** for PR 31273 at commit [`9432fec`](https://github.com/apache/spark/commit/9432fec05a7c2ca1e83c6811d12db343e9e56015).


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

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

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r561610487



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -1363,9 +1368,14 @@ class SessionCatalog(
       Utils.classForName("org.apache.spark.sql.expressions.UserDefinedAggregateFunction")
     if (clsForUDAF.isAssignableFrom(clazz)) {
       val cls = Utils.classForName("org.apache.spark.sql.execution.aggregate.ScalaUDAF")
-      val e = cls.getConstructor(classOf[Seq[Expression]], clsForUDAF, classOf[Int], classOf[Int])
-        .newInstance(input,
-          clazz.getConstructor().newInstance().asInstanceOf[Object], Int.box(1), Int.box(1))
+      val e = cls.getConstructor(

Review comment:
       There are some UDAF related change, is it related to the view stuff here?

##########
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:
       +1 if it works, adding tag seems hacky.




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770305305


   **[Test build #134683 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134683/testReport)** for PR 31273 at commit [`ac663aa`](https://github.com/apache/spark/commit/ac663aa0dc50f129ab0f65d2723d76c6a3e97b3c).


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-764357237


   **[Test build #134304 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134304/testReport)** for PR 31273 at commit [`71c01e8`](https://github.com/apache/spark/commit/71c01e85c7eb9a530607078730dd5cb8bc533be6).


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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778694227


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135140/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-767177034






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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784809732


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39985/
   


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r579493961



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -862,7 +862,13 @@ class Analyzer(override val catalogManager: CatalogManager)
   }
 
   private def isResolvingView: Boolean = AnalysisContext.get.catalogAndNamespace.nonEmpty
-  private def referredTempViewNames: Seq[Seq[String]] = AnalysisContext.get.referredTempViewNames
+  private def isReferredTempViewName(nameParts: Seq[String]): Boolean = {
+    AnalysisContext.get.referredTempViewNames.exists { n =>
+      (n.length == nameParts.length) && n.zip(nameParts).forall {

Review comment:
       That doesn't compile?
   ```
   [error]  found   : org.apache.spark.sql.catalyst.analysis.Resolver
   [error]     (which expands to)  (String, String) => Boolean
   [error]  required: ((String, String)) => Boolean
   [error]       (n.length == nameParts.length) && n.zip(nameParts).forall(resolver)
   ```

##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -541,40 +539,33 @@ object ViewHelper {
   }
 
   /**
-   * Collect all temporary views and functions and return the identifiers separately
-   * This func traverses the unresolved plan `child`. Below are the reasons:

Review comment:
       This comment is not relevant any more since analyzed plan is now used.

##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -152,10 +152,10 @@ case class CreateViewCommand(
             sparkSession,
             analyzedPlan,
             aliasedPlan.schema,
-            originalText,
-            child))
+            originalText))
       } else {
-        aliasedPlan
+        assert(isTemporary)

Review comment:
       Removed.

##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -152,10 +152,10 @@ case class CreateViewCommand(
             sparkSession,
             analyzedPlan,
             aliasedPlan.schema,
-            originalText,
-            child))
+            originalText))
       } else {
-        aliasedPlan
+        assert(isTemporary)
+        View(None, isTemporary, aliasedPlan)

Review comment:
       Thanks, moved to `SessionCatalog .getTempViewPlan`.

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
##########
@@ -143,6 +143,7 @@ abstract class Optimizer(catalogManager: CatalogManager)
       EliminateResolvedHint,
       EliminateSubqueryAliases,
       EliminateView,
+      EliminateDataFrameTempViews,

Review comment:
       Thanks, removed.

##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -129,10 +129,10 @@ case class CreateViewCommand(
             sparkSession,
             analyzedPlan,
             aliasedPlan.schema,
-            originalText,
-            child))
+            originalText))
       } else {
-        aliasedPlan
+        assert(isTemporary)

Review comment:
       Removed.

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -840,14 +840,9 @@ class SessionCatalog(
     plan match {
       case viewInfo: TemporaryViewRelation =>
         fromCatalogTable(viewInfo.tableMeta, isTempView = true)
-      case v => v
-    }
-  }
-
-  def getTempViewSchema(plan: LogicalPlan): StructType = {

Review comment:
       This def was not being used, so I am removing it.

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -923,9 +933,9 @@ class Analyzer(override val catalogManager: CatalogManager)
     def lookupTempView(
         identifier: Seq[String], isStreaming: Boolean = false): Option[LogicalPlan] = {
       // Permanent View can't refer to temp views, no need to lookup at all.
-      if (isResolvingView && !referredTempViewNames.contains(identifier)) return None
+      if (isResolvingView && !isReferredTempViewName(identifier)) return None
 
-      val tmpView = identifier match {
+      val tmpView = expandRelationName(identifier) match {

Review comment:
       Thanks, can be removed now (after `isReferredTempViewName` fix)




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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-781937243


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135254/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778665667


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39721/
   


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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778584742


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135135/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778559273


   **[Test build #135130 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135130/testReport)** for PR 31273 at commit [`fbdcf3b`](https://github.com/apache/spark/commit/fbdcf3b7535392ef0cd02d7ab32cbb3e5f8116b4).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784722249


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39972/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782441998


   **[Test build #135282 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135282/testReport)** for PR 31273 at commit [`4e55a1f`](https://github.com/apache/spark/commit/4e55a1f4fac4720782670aafe3e2fa3f9252ec6b).
    * This patch **fails to build**.
    * This patch merges cleanly.
    * This patch adds no public classes.


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r578253558



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala
##########
@@ -443,33 +443,38 @@ case class InsertIntoDir(
 }
 
 /**
- * A container for holding the view description(CatalogTable), and the output of the view. The
- * child should be a logical plan parsed from the `CatalogTable.viewText`, should throw an error
- * if the `viewText` is not defined.
+ * A container for holding the view description(CatalogTable) and info whether the view is temporary
+ * or not. If the view description is available, the child should be a logical plan parsed from the
+ * `CatalogTable.viewText`. Otherwise, the view is a temporary one created from a dataframe; in this
+ * case, the child must be already resolved.

Review comment:
       hmm does it really matter? `View.child` is visible to analyzer rules and will be analyzed eventually.




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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782437490






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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-781634014


   **[Test build #135244 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135244/testReport)** for PR 31273 at commit [`bace91a`](https://github.com/apache/spark/commit/bace91a680e093ef4828495ee16a931769f2aca2).


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784976433


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135406/
   


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r567838659



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/CacheManager.scala
##########
@@ -159,11 +159,19 @@ class CacheManager extends Logging with AdaptiveSparkPlanHelper {
       plan: LogicalPlan,
       cascade: Boolean,
       blocking: Boolean = false): Unit = {
+    val analyzedPlan = if (plan.resolved) {
+      plan
+    } else {
+      val qe = spark.sessionState.executePlan(plan)
+      qe.assertAnalyzed()

Review comment:
       where do we call `uncacheQuery` with an unresolved plan?




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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-764403536


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134304/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782591841


   **[Test build #135293 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135293/testReport)** for PR 31273 at commit [`9da3b9c`](https://github.com/apache/spark/commit/9da3b9cd6ff1b6080caf0cfa7ab0021b0a079879).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


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

[GitHub] [spark] cloud-fan 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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r563510829



##########
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:
       `Seq(1).map(i => (i, i)).toDF("i", "i").createTempView("tv1")` is not SQL temp view (but dataframe temp view), so it's still allowed to have duplicated column names.




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-765884015


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38982/
   


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r561604269



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala
##########
@@ -2029,7 +2029,7 @@ class DataSourceV2SQLSuite
   test("CREATE VIEW") {
     val v = "testcat.ns1.ns2.v"
     val e = intercept[AnalysisException] {
-      sql(s"CREATE VIEW $v AS SELECT * FROM tab1")
+      sql(s"CREATE VIEW $v AS SELECT 1")

Review comment:
       This needs to be updated now that `ResolveSessionCatalog` handles `CreateViewStatement` only if its `child` is resolved.

##########
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:
       This seems a bit hacky, but `SubqueryAlias(_, view: View)` couldn't handle all the cases. For example,
   ```scala
   spark.range(10).createTempView("t")
   sql("CREATE VIEW v AS SELECT * FROM t")
   ```
   The `child` is:
   ```
   Project [id#16L]
   +- SubqueryAlias t
      +- Range (0, 10, step=1, splits=Some(2))
   ```
   @cloud-fan, @viirya Any suggestion to capture the view properly? Does it make sense to add an `isView` field to `SubqueryAlias`?




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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782568360


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39872/
   


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r561607878



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -1086,7 +1086,7 @@ class Analyzer(override val catalogManager: CatalogManager)
         }
         view.copy(child = newChild)
       case p @ SubqueryAlias(_, view: View) =>
-        p.copy(child = resolveViews(view))
+        p.makeCopy(Array(p.identifier, resolveViews(view)))

Review comment:
       This is to copy the tags, but seems hacky since `copy` can be called to remove the tags in other places. Please check https://github.com/apache/spark/pull/31273#discussion_r561607183 if it makes sense to add `isView` to `SubqueryAlias`.




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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784821270


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39985/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778690769


   **[Test build #135140 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135140/testReport)** for PR 31273 at commit [`e92cbe1`](https://github.com/apache/spark/commit/e92cbe1427dacb8f6b1f7e967c4d1c6a75c32ebf).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580757641



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -838,9 +836,11 @@ class SessionCatalog(
 
   private def getTempViewPlan(plan: LogicalPlan): LogicalPlan = {
     plan match {
-      case viewInfo: TemporaryViewRelation =>
-        fromCatalogTable(viewInfo.tableMeta, isTempView = true)
-      case v => v
+      case TemporaryViewRelation(tableMeta, None) =>
+        fromCatalogTable(tableMeta, isTempView = true)
+      case TemporaryViewRelation(tableMeta, Some(plan)) =>
+        View(desc = tableMeta, isTempView = true, child = plan)
+      case other => other

Review comment:
       If the current PR's approach is fine, we can remove this `case other => other` and change `createTempView` to use `tableDefinition: TemporaryViewRelation` instead of `tableDefinition: LogicalPlan` once we migrate `ALTER VIEW AS` and `CREATE TEMP VIEW USING`.




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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r561612294



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -1363,9 +1368,14 @@ class SessionCatalog(
       Utils.classForName("org.apache.spark.sql.expressions.UserDefinedAggregateFunction")
     if (clsForUDAF.isAssignableFrom(clazz)) {
       val cls = Utils.classForName("org.apache.spark.sql.execution.aggregate.ScalaUDAF")
-      val e = cls.getConstructor(classOf[Seq[Expression]], clsForUDAF, classOf[Int], classOf[Int])
-        .newInstance(input,
-          clazz.getConstructor().newInstance().asInstanceOf[Object], Int.box(1), Int.box(1))
+      val e = cls.getConstructor(

Review comment:
       Yea, we need to collect temp functions by name. Currently, UDAF doesn't store a name, so I added 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.

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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-767181349


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39051/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784722249


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39972/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-769415691


   **[Test build #134629 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134629/testReport)** for PR 31273 at commit [`bfabe9f`](https://github.com/apache/spark/commit/bfabe9f60e472d058aca3fc9837431c960f9380c).


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770562692


   **[Test build #134699 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134699/testReport)** for PR 31273 at commit [`60cf521`](https://github.com/apache/spark/commit/60cf5211fe0aee7d31c47a6c8fd269b3ca9fdd1d).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r579007724



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -152,10 +152,10 @@ case class CreateViewCommand(
             sparkSession,
             analyzedPlan,
             aliasedPlan.schema,
-            originalText,
-            child))
+            originalText))
       } else {
-        aliasedPlan
+        assert(isTemporary)

Review comment:
       ditto




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770324692


   **[Test build #134685 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134685/testReport)** for PR 31273 at commit [`0fbf1cf`](https://github.com/apache/spark/commit/0fbf1cf58032e923e468c5c51c165535d8d7e690).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770337439


   **[Test build #134688 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134688/testReport)** for PR 31273 at commit [`4e77301`](https://github.com/apache/spark/commit/4e77301c382807f9f2c775f9e59e0798b0dd0548).


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r568167394



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/CacheManager.scala
##########
@@ -159,11 +159,19 @@ class CacheManager extends Logging with AdaptiveSparkPlanHelper {
       plan: LogicalPlan,
       cascade: Boolean,
       blocking: Boolean = false): Unit = {
+    val analyzedPlan = if (plan.resolved) {
+      plan
+    } else {
+      val qe = spark.sessionState.executePlan(plan)
+      qe.assertAnalyzed()

Review comment:
       ```
   sql("CACHE TABLE t1 AS SELECT * FROM testData")
   sql("UNCACHE TABLE t1")
   ```
   `t1` passed to `uncacheQuery` is:
   ```
   'SubqueryAlias t1
   +- View (`t1`, ['key,'value])
      +- 'Project [upcast('key, IntegerType) AS key#293, upcast('value, StringType) AS value#294]
         +- 'Project [*]
            +- 'UnresolvedRelation [testData], [], false
   ```
   Now that temporary views are always resolved, we need to resolve `t1` in this case.
   
   Note that we couldn't make the table of `CacheTable`/`UncacheTable` as a child due to https://github.com/apache/spark/pull/30598#discussion_r536384428.
   




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

[GitHub] [spark] SparkQA removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782557636


   **[Test build #135293 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135293/testReport)** for PR 31273 at commit [`9da3b9c`](https://github.com/apache/spark/commit/9da3b9cd6ff1b6080caf0cfa7ab0021b0a079879).


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580815988



##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala
##########
@@ -301,16 +307,16 @@ abstract class SessionCatalogSuite extends AnalysisTest with Eventually {
       val tempTable2 = Range(1, 20, 2, 10)
       catalog.createTempView("tbl1", tempTable1, overrideIfExists = false)
       catalog.createTempView("tbl2", tempTable2, overrideIfExists = false)
-      assert(catalog.getTempView("tbl1") == Option(tempTable1))
-      assert(catalog.getTempView("tbl2") == Option(tempTable2))
-      assert(catalog.getTempView("tbl3").isEmpty)
+      assert(getTempViewRawPlan(catalog.getTempView("tbl1")) == Option(tempTable1))

Review comment:
       Shall we test the actual plan with the View wrapper? To make sure the generated `CatalogTable` is correct.




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778564012


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39716/
   


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-783861974


   Thanks @cloud-fan for the suggestion. I pushed new changes that do not require `View` signature changes. Basically, the following changes are needed:
   1. `TemporaryViewRelation` now stores an optional analyzed plan if the temp view is created from dataframe.
   2. `CatalogTable`'s properties now stores `VIEW_CREATED_FROM_DATAFRAME` as a key to to differentiate the view create from dataframe.
   
   (On a side note, I first made a change without doing 1), by creating `CatalogTable` on the fly in `SessionCatalog`. However, there were tests that compare analyzed plans which were failing due to `CatalogTable.createTime` difference, so I moved creating `CatalogTable` in `CreateViewCommand.run`. It seems that this is a better approach where we can wrap all the temporary views with `TemporaryViewRelation` eventually once we also migrate `ALTER VIEW` and `CREATE TEMP VIEW USING`.)


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

[GitHub] [spark] SparkQA removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778442948


   **[Test build #135127 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135127/testReport)** for PR 31273 at commit [`7e92eeb`](https://github.com/apache/spark/commit/7e92eeb88aebba1fae2e6e94bc4e678998118fea).


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770607734


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39293/
   


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r579007420



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -129,10 +129,10 @@ case class CreateViewCommand(
             sparkSession,
             analyzedPlan,
             aliasedPlan.schema,
-            originalText,
-            child))
+            originalText))
       } else {
-        aliasedPlan
+        assert(isTemporary)

Review comment:
       we are in the branch of `if (viewType == LocalTempView)`, this assert seems not necessary, as the if condition is not far away.




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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-765881666


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134396/
   


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

[GitHub] [spark] cloud-fan closed pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #31273:
URL: https://github.com/apache/spark/pull/31273


   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778694227


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135140/
   


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

[GitHub] [spark] SparkQA removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778558253


   **[Test build #135135 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135135/testReport)** for PR 31273 at commit [`2a3e5b7`](https://github.com/apache/spark/commit/2a3e5b74cfa5277fd3b72db38b205901474294e6).


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

[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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r563932272



##########
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 will try 2) if there is no opposition to the approach.




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

[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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r563930489



##########
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:
       OK. In that case, we are left with two options:
   1) Add `isView: Boolean` field to the `SubqueryAlias`, or
   2) Introduce a new logical plan for handling dataframe temp view.
   




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

[GitHub] [spark] SparkQA removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-781634014


   **[Test build #135244 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135244/testReport)** for PR 31273 at commit [`bace91a`](https://github.com/apache/spark/commit/bace91a680e093ef4828495ee16a931769f2aca2).


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782568360


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39872/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-781715027






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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770369373


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134688/
   


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r581589456



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala
##########
@@ -443,21 +444,25 @@ case class InsertIntoDir(
 }
 
 /**
- * A container for holding the view description(CatalogTable), and the output of the view. The
- * child should be a logical plan parsed from the `CatalogTable.viewText`, should throw an error
- * if the `viewText` is not defined.
+ * A container for holding the view description(CatalogTable) and info whether the view is temporary
+ * or not. If the view description is available, the child should be a logical plan parsed from the

Review comment:
       `If the view description is available`, it's always available now.




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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782592079


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135293/
   


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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580758902



##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/ResolveHintsSuite.scala
##########
@@ -30,37 +30,48 @@ import org.apache.spark.sql.types.IntegerType
 class ResolveHintsSuite extends AnalysisTest {
   import org.apache.spark.sql.catalyst.analysis.TestRelations._
 
+  private def checkAnalysisWithTempViewEliminated(
+      inputPlan: LogicalPlan,
+      expectedPlan: LogicalPlan,
+      caseSensitive: Boolean = true): Unit = {
+    checkAnalysisWithTransform(inputPlan, expectedPlan, caseSensitive) { plan =>
+      plan transformUp {
+        case v: View if v.isDataFrameTempView => v.child

Review comment:
       Now that we require to create `CatalogTable`, we just remove `View` node since we need to synchronize `CatalogTable.createTime`.




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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580817158



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -489,8 +500,8 @@ object ViewHelper {
       path: Seq[TableIdentifier],
       viewIdent: TableIdentifier): Unit = {
     plan match {
-      case v: View =>
-        val ident = v.desc.identifier
+      case v @ View(desc, _, _) =>

Review comment:
       unnecessary change.




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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770763435


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134707/
   


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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784823052


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135392/
   


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r581589456



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala
##########
@@ -443,21 +444,25 @@ case class InsertIntoDir(
 }
 
 /**
- * A container for holding the view description(CatalogTable), and the output of the view. The
- * child should be a logical plan parsed from the `CatalogTable.viewText`, should throw an error
- * if the `viewText` is not defined.
+ * A container for holding the view description(CatalogTable) and info whether the view is temporary
+ * or not. If the view description is available, the child should be a logical plan parsed from the

Review comment:
       `If the view description is available`, it's always available now. should be `if it's not a dataframe temp view`




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-782437553


   **[Test build #135282 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135282/testReport)** for PR 31273 at commit [`4e55a1f`](https://github.com/apache/spark/commit/4e55a1f4fac4720782670aafe3e2fa3f9252ec6b).


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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580813358



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -838,9 +836,11 @@ class SessionCatalog(
 
   private def getTempViewPlan(plan: LogicalPlan): LogicalPlan = {
     plan match {
-      case viewInfo: TemporaryViewRelation =>
-        fromCatalogTable(viewInfo.tableMeta, isTempView = true)
-      case v => v
+      case TemporaryViewRelation(tableMeta, None) =>
+        fromCatalogTable(tableMeta, isTempView = true)
+      case TemporaryViewRelation(tableMeta, Some(plan)) =>
+        View(desc = tableMeta, isTempView = true, child = plan)
+      case other => other

Review comment:
       +1




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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778660542


   **[Test build #135140 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135140/testReport)** for PR 31273 at commit [`e92cbe1`](https://github.com/apache/spark/commit/e92cbe1427dacb8f6b1f7e967c4d1c6a75c32ebf).


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770348002


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39274/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778567456


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39716/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-769469354






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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580818323



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
##########
@@ -541,40 +552,31 @@ object ViewHelper {
   }
 
   /**
-   * Collect all temporary views and functions and return the identifiers separately
-   * This func traverses the unresolved plan `child`. Below are the reasons:
-   * 1) Analyzer replaces unresolved temporary views by a SubqueryAlias with the corresponding
-   * logical plan. After replacement, it is impossible to detect whether the SubqueryAlias is
-   * added/generated from a temporary view.
-   * 2) The temp functions are represented by multiple classes. Most are inaccessible from this
-   * package (e.g., HiveGenericUDF).
+   * Collect all temporary views and functions and return the identifiers separately.
    */
   private def collectTemporaryObjects(
       catalog: SessionCatalog, child: LogicalPlan): (Seq[Seq[String]], Seq[String]) = {
     def collectTempViews(child: LogicalPlan): Seq[Seq[String]] = {
       child.flatMap {
-        case UnresolvedRelation(nameParts, _, _) if catalog.isTempView(nameParts) =>
-          Seq(nameParts)
-        case w: With if !w.resolved => w.innerChildren.flatMap(collectTempViews)
-        case plan if !plan.resolved => plan.expressions.flatMap(_.flatMap {
+        case s @ SubqueryAlias(_, view: View) if view.isTempView =>

Review comment:
       ah it's `child.flatMap`, seems we don't need to match `SubqueryAlias` here?




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

[GitHub] [spark] cloud-fan 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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r563510450



##########
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 think it's expected. The goal is to make SQL temp view and permanent view consistent, and forbidding duplicated column names is consistent.

##########
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:
       `Seq(1).map(i => (i, i)).toDF("i", "i").createTempView("tv1")` is not SQL temp view (but dataframe temp view), so it's still allowed to have duplicated column names.

##########
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:
       Shall we not wrap dataframe temp view with `View`? The original goal is to make SQL temp view and permanent view consistent.

##########
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:
       OK I get your point now. If we need a new logical plan, then it's better to reuse `View` with some modification. +1 to make `View.desc` optional and set it to None for dataframe temp view. Or we can generate column names for dataframe temp view in `View.desc`, like `col1, col2, ...`, to bypass the check.




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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770560937






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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778442948


   **[Test build #135127 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135127/testReport)** for PR 31273 at commit [`7e92eeb`](https://github.com/apache/spark/commit/7e92eeb88aebba1fae2e6e94bc4e678998118fea).


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770547652


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39285/
   


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778530167


   **[Test build #135127 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135127/testReport)** for PR 31273 at commit [`7e92eeb`](https://github.com/apache/spark/commit/7e92eeb88aebba1fae2e6e94bc4e678998118fea).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


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

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

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-784975137


   **[Test build #135406 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135406/testReport)** for PR 31273 at commit [`82d58ba`](https://github.com/apache/spark/commit/82d58baa98235f912141b2b40cdcd3bf0bdbe744).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-764403536






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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-783901727






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

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

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580813778



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala
##########
@@ -443,21 +444,23 @@ case class InsertIntoDir(
 }
 
 /**
- * A container for holding the view description(CatalogTable), and the output of the view. The
- * child should be a logical plan parsed from the `CatalogTable.viewText`, should throw an error
- * if the `viewText` is not defined.
+ * A container for holding the view description(CatalogTable) and info whether the view is temporary
+ * or not. If the view description is available, the child should be a logical plan parsed from the
+ * `CatalogTable.viewText`. Otherwise, the view is a temporary one created from a dataframe; in this

Review comment:
       This doc needs an update now.




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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r580758321



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -683,7 +681,7 @@ class SessionCatalog(
     val table = formatTableName(name.table)
     if (name.database.isEmpty) {
       tempViews.get(table).map {
-        case TemporaryViewRelation(metadata) => metadata
+        case TemporaryViewRelation(metadata, _) => metadata
         case plan =>

Review comment:
       If https://github.com/apache/spark/pull/31273/files#r580757641 is done, `case plan` can also be removed.




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

[GitHub] [spark] AmplabJenkins removed a comment on pull request #31273: [SPARK-34152][SQL] Make CreateViewStatement.child to be LogicalPlan's children so that it's resolved in analyze phase

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-783901727






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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-770324995






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

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

Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r575511343



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/CacheManager.scala
##########
@@ -159,11 +159,19 @@ class CacheManager extends Logging with AdaptiveSparkPlanHelper {
       plan: LogicalPlan,
       cascade: Boolean,
       blocking: Boolean = false): Unit = {
+    val analyzedPlan = if (plan.resolved) {
+      plan
+    } else {
+      val qe = spark.sessionState.executePlan(plan)
+      qe.assertAnalyzed()

Review comment:
       Actually, it was subtle why `UnresolvedRelation [testData]` inside temp view `t1` was not resolved.
   When the temp view `t1` is looked up, its text is parsed and produces:
   ```
   'SubqueryAlias t1
   +- View (`t1`, ['key,'value])
      +- 'Project [upcast('key, IntegerType) AS key#293, upcast('value, StringType) AS value#294]
         +- 'Project [*]
            +- 'UnresolvedRelation [testData], [], false
   ```
   Now this logical plan goes thru the following to resolve this temp view further: https://github.com/apache/spark/blob/e0053853c90d39ef6de9d59fb933525e20bae1fa/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala#L1100
   
   However, we have the following check that doesn't allow resolving further:
   https://github.com/apache/spark/blob/e0053853c90d39ef6de9d59fb933525e20bae1fa/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala#L924-L925
   
   But, we are resolving a temp view in this context, so I updated the check to allow relations in temp views are resolved correctly.




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

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

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31273:
URL: https://github.com/apache/spark/pull/31273#discussion_r561621637



##########
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:
       +1 if it works, adding tag seems hacky.




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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-781848158


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39834/
   


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

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

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31273:
URL: https://github.com/apache/spark/pull/31273#issuecomment-778468902


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39708/
   


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