You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gatorsmile <gi...@git.apache.org> on 2016/11/04 04:31:25 UTC

[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

GitHub user gatorsmile opened a pull request:

    https://github.com/apache/spark/pull/15764

    [SPARK-18217] [SQL] Disallow creating permanent views based on temporary views or UDFs

    ### What changes were proposed in this pull request?
    Based on the discussion in [SPARK-18209](https://issues.apache.org/jira/browse/SPARK-18209). It doesn't really make sense to create permanent views based on temporary views or temporary UDFs. 
    
    This PR is to disallow the supports and issue the exceptions.
    
    TODO: update the PR descriptions.
    
    ### How was this patch tested?
    Added test cases.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/gatorsmile/spark blockTempFromPermViewCreation

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/15764.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #15764
    
----
commit 509327e303984906ce4c87e459d6323582963089
Author: gatorsmile <ga...@gmail.com>
Date:   2016-11-04T02:42:18Z

    fix.

commit 1b430bb366d1bd2e567f87c9b640e7c88ffbb144
Author: gatorsmile <ga...@gmail.com>
Date:   2016-11-04T04:25:18Z

    fix2

commit 695110f35df363286aad015ca591289e171ed185
Author: gatorsmile <ga...@gmail.com>
Date:   2016-11-04T04:28:00Z

    update code comments

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    **[Test build #68116 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68116/consoleFull)** for PR 15764 at commit [`86e7f9d`](https://github.com/apache/spark/commit/86e7f9d8afdaed72666b79692a31a9131461dc29).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86493888
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
    @@ -923,6 +923,24 @@ class SessionCatalog(
         }
       }
     
    +  /**
    +   * Returns whether it is a temporary function.
    +   */
    +  def isTemporaryFunction(name: FunctionIdentifier): Boolean = {
    --- End diff --
    
    also what's the behavior if the function doesn't exist?



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86569498
  
    --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala ---
    @@ -462,4 +487,65 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
           }
         }
       }
    +
    +  test("create a permanent/temp view using a hive function") {
    --- End diff --
    
    Sure, will do it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86586265
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
    @@ -923,6 +923,24 @@ class SessionCatalog(
         }
       }
     
    +  /**
    +   * Returns whether it is a temporary function.
    +   */
    +  def isTemporaryFunction(name: FunctionIdentifier): Boolean = {
    --- End diff --
    
    Like `isTemporaryTable`, we return false when the function/table does not exist


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86659535
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
    @@ -173,6 +177,34 @@ case class CreateViewCommand(
       }
     
       /**
    +   * Permanent views are not allowed to reference temp objects, including temp function and views
    +   */
    +  private def verifyTemporaryObjectsNotExists(sparkSession: SparkSession): Unit = {
    +    if (!isTemporary) {
    +      // This func traverses the unresolved plan `child`. Below are the reasons:
    --- End diff --
    
    is it possible the `child` is resolved? e.g. by `DataFrame` API


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    Looks good but I didn't read super carefully.
    
    cc @hvanhovell and @cloud-fan 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86493701
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
    @@ -131,6 +131,26 @@ case class CreateViewCommand(
             s"specified by CREATE VIEW (num: `${userSpecifiedColumns.length}`).")
         }
     
    +    // When creating a permanent view, not allowed to reference temporary objects.
    +    if (!isTemporary) {
    +      child.collect {
    +        // Disallow creating permanent views based on temporary views.
    +        case s: UnresolvedRelation
    +            if sparkSession.sessionState.catalog.isTemporaryTable(s.tableIdentifier) =>
    +          throw new AnalysisException(s"Not allowed to create a permanent view $name by " +
    +            s"referencing a temp view ${s.tableIdentifier}. " +
    +            originalText.map(sql => s"""SQL: "$sql".""").getOrElse(""))
    +        case other if !other.resolved => other.expressions.flatMap(_.collect {
    +          // Disallow creating permanent views based on temporary UDFs.
    +          case e: UnresolvedFunction
    +              if sparkSession.sessionState.catalog.isTemporaryFunction(e.name) =>
    +            throw new AnalysisException(s"Not allowed to create a permanent view $name by " +
    +              s"referencing a temp function `${e.name}`. " +
    --- End diff --
    
    temp -> temporary


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86497413
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
    @@ -923,6 +923,24 @@ class SessionCatalog(
         }
       }
     
    +  /**
    +   * Returns whether it is a temporary function.
    +   */
    +  def isTemporaryFunction(name: FunctionIdentifier): Boolean = {
    --- End diff --
    
    Will resolve this tomorrow. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/68247/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86659525
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
    @@ -923,6 +923,24 @@ class SessionCatalog(
         }
       }
     
    +  /**
    +   * Returns whether it is a temporary function. If not existed, returns false.
    +   */
    +  def isTemporaryFunction(name: FunctionIdentifier): Boolean = {
    +    // copied from HiveSessionCatalog
    +    val hiveFunctions = Seq(
    +      "hash",
    +      "histogram_numeric",
    +      "percentile")
    +
    +    // A temporary function is a function that has been registered in functionRegistry
    +    // without a database name, and is neither a built-in function nor a Hive function
    +    name.database.isEmpty &&
    +      functionRegistry.functionExists(name.funcName) &&
    +      !FunctionRegistry.builtin.functionExists(name.funcName) &&
    --- End diff --
    
    `toLowerCase`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    **[Test build #68112 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68112/consoleFull)** for PR 15764 at commit [`7100a8f`](https://github.com/apache/spark/commit/7100a8fcb3975d5f00fcc9e485f3606873fd1a0d).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    **[Test build #68247 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68247/consoleFull)** for PR 15764 at commit [`fec0066`](https://github.com/apache/spark/commit/fec0066331aaf085f8cd0318be23844560e72b4b).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86491722
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala ---
    @@ -709,10 +709,13 @@ case class LocalLimit(limitExpr: Expression, child: LogicalPlan) extends UnaryNo
     case class SubqueryAlias(
         alias: String,
         child: LogicalPlan,
    -    view: Option[TableIdentifier])
    +    view: Option[TableIdentifier])(
    +    val isGeneratedByTempTable: java.lang.Boolean = false)
    --- End diff --
    
    Maybe we can do it from the unanalyzed plan, like what this PR did for temp functions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    **[Test build #68143 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68143/consoleFull)** for PR 15764 at commit [`a4df82b`](https://github.com/apache/spark/commit/a4df82b255089e9daf4167846186f25879a60da3).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86689510
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
    @@ -923,6 +923,24 @@ class SessionCatalog(
         }
       }
     
    +  /**
    +   * Returns whether it is a temporary function. If not existed, returns false.
    +   */
    +  def isTemporaryFunction(name: FunctionIdentifier): Boolean = {
    +    // copied from HiveSessionCatalog
    +    val hiveFunctions = Seq(
    +      "hash",
    +      "histogram_numeric",
    +      "percentile")
    +
    +    // A temporary function is a function that has been registered in functionRegistry
    +    // without a database name, and is neither a built-in function nor a Hive function
    +    name.database.isEmpty &&
    +      functionRegistry.functionExists(name.funcName) &&
    +      !FunctionRegistry.builtin.functionExists(name.funcName) &&
    --- End diff --
    
    Our built-in function registry is using `SimpleFunctionRegistry`, which is using [case insensitive string key hash map](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala#L71-L72). 
    
    Thus, no need to add `toLowerCase`. However, we need to add a test case to ensure we can capture the potential change in this part.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    Thanks - merging in master/branch-2.1.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    **[Test build #68161 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68161/consoleFull)** for PR 15764 at commit [`1c3899f`](https://github.com/apache/spark/commit/1c3899f8092d2f568391423df27cac77ee58ffca).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    **[Test build #68143 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68143/consoleFull)** for PR 15764 at commit [`a4df82b`](https://github.com/apache/spark/commit/a4df82b255089e9daf4167846186f25879a60da3).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/68112/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86659555
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
    @@ -173,6 +177,34 @@ case class CreateViewCommand(
       }
     
       /**
    +   * Permanent views are not allowed to reference temp objects, including temp function and views
    +   */
    +  private def verifyTemporaryObjectsNotExists(sparkSession: SparkSession): Unit = {
    +    if (!isTemporary) {
    +      // This func traverses the unresolved plan `child`. Below are the reasons:
    --- End diff --
    
    nvm, we can only create permanent view by SQL API


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86493730
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
    @@ -131,6 +131,26 @@ case class CreateViewCommand(
             s"specified by CREATE VIEW (num: `${userSpecifiedColumns.length}`).")
         }
     
    +    // When creating a permanent view, not allowed to reference temporary objects.
    +    if (!isTemporary) {
    +      child.collect {
    +        // Disallow creating permanent views based on temporary views.
    +        case s: UnresolvedRelation
    +            if sparkSession.sessionState.catalog.isTemporaryTable(s.tableIdentifier) =>
    +          throw new AnalysisException(s"Not allowed to create a permanent view $name by " +
    +            s"referencing a temp view ${s.tableIdentifier}. " +
    +            originalText.map(sql => s"""SQL: "$sql".""").getOrElse(""))
    +        case other if !other.resolved => other.expressions.flatMap(_.collect {
    +          // Disallow creating permanent views based on temporary UDFs.
    +          case e: UnresolvedFunction
    +              if sparkSession.sessionState.catalog.isTemporaryFunction(e.name) =>
    +            throw new AnalysisException(s"Not allowed to create a permanent view $name by " +
    +              s"referencing a temp function `${e.name}`. " +
    +              originalText.map(sql => s"""SQL: "$sql".""").getOrElse(""))
    --- End diff --
    
    same here do we really need to show the entire sql? it's basically what the user just typed in


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86491162
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
    @@ -923,6 +923,24 @@ class SessionCatalog(
         }
       }
     
    +  /**
    +   * Returns whether it is a temporary function.
    +   */
    +  def isTempFunction(name: FunctionIdentifier): Boolean = {
    +    // copied from HiveSessionCatalog
    --- End diff --
    
    Will do. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/15764


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    **[Test build #68247 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68247/consoleFull)** for PR 15764 at commit [`fec0066`](https://github.com/apache/spark/commit/fec0066331aaf085f8cd0318be23844560e72b4b).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/68116/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    **[Test build #68161 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68161/consoleFull)** for PR 15764 at commit [`1c3899f`](https://github.com/apache/spark/commit/1c3899f8092d2f568391423df27cac77ee58ffca).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86489677
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
    @@ -923,6 +923,24 @@ class SessionCatalog(
         }
       }
     
    +  /**
    +   * Returns whether it is a temporary function.
    +   */
    +  def isTempFunction(name: FunctionIdentifier): Boolean = {
    +    // copied from HiveSessionCatalog
    --- End diff --
    
    i'd update HiveSessionCatalog to say don't forget to update this place. Otherwise it will be inconsistent.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86613372
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
    @@ -923,6 +923,24 @@ class SessionCatalog(
         }
       }
     
    +  /**
    +   * Returns whether it is a temporary function.
    +   */
    +  def isTemporaryFunction(name: FunctionIdentifier): Boolean = {
    --- End diff --
    
    Yea please docuemnt it.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86695651
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
    @@ -923,6 +923,24 @@ class SessionCatalog(
         }
       }
     
    +  /**
    +   * Returns whether it is a temporary function. If not existed, returns false.
    +   */
    +  def isTemporaryFunction(name: FunctionIdentifier): Boolean = {
    +    // copied from HiveSessionCatalog
    +    val hiveFunctions = Seq(
    +      "hash",
    +      "histogram_numeric",
    +      "percentile")
    +
    +    // A temporary function is a function that has been registered in functionRegistry
    +    // without a database name, and is neither a built-in function nor a Hive function
    +    name.database.isEmpty &&
    +      functionRegistry.functionExists(name.funcName) &&
    +      !FunctionRegistry.builtin.functionExists(name.funcName) &&
    +      !hiveFunctions.contains(name.funcName.toLowerCase)
    --- End diff --
    
    It is true - but we are working towards getting rid of HiveSessionCatalog though (including getting rid of the 3 fallback functions), so in practice this will make no difference soon.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86493685
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
    @@ -131,6 +131,26 @@ case class CreateViewCommand(
             s"specified by CREATE VIEW (num: `${userSpecifiedColumns.length}`).")
         }
     
    +    // When creating a permanent view, not allowed to reference temporary objects.
    +    if (!isTemporary) {
    +      child.collect {
    +        // Disallow creating permanent views based on temporary views.
    +        case s: UnresolvedRelation
    +            if sparkSession.sessionState.catalog.isTemporaryTable(s.tableIdentifier) =>
    +          throw new AnalysisException(s"Not allowed to create a permanent view $name by " +
    +            s"referencing a temp view ${s.tableIdentifier}. " +
    --- End diff --
    
    temp -> temporary


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/68161/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86493863
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
    @@ -923,6 +923,24 @@ class SessionCatalog(
         }
       }
     
    +  /**
    +   * Returns whether it is a temporary function.
    +   */
    +  def isTemporaryFunction(name: FunctionIdentifier): Boolean = {
    --- End diff --
    
    add a unit test for this function?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/68143/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86494337
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
    @@ -131,6 +131,26 @@ case class CreateViewCommand(
             s"specified by CREATE VIEW (num: `${userSpecifiedColumns.length}`).")
         }
     
    +    // When creating a permanent view, not allowed to reference temporary objects.
    +    if (!isTemporary) {
    +      child.collect {
    +        // Disallow creating permanent views based on temporary views.
    --- End diff --
    
    i'd also copy paste what you put in the pr description in here, on why you are traversing the unresolved plan.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/68107/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    **[Test build #68107 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68107/consoleFull)** for PR 15764 at commit [`695110f`](https://github.com/apache/spark/commit/695110f35df363286aad015ca591289e171ed185).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    **[Test build #68112 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68112/consoleFull)** for PR 15764 at commit [`7100a8f`](https://github.com/apache/spark/commit/7100a8fcb3975d5f00fcc9e485f3606873fd1a0d).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    **[Test build #68107 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68107/consoleFull)** for PR 15764 at commit [`695110f`](https://github.com/apache/spark/commit/695110f35df363286aad015ca591289e171ed185).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86691491
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
    @@ -923,6 +923,24 @@ class SessionCatalog(
         }
       }
     
    +  /**
    +   * Returns whether it is a temporary function. If not existed, returns false.
    +   */
    +  def isTemporaryFunction(name: FunctionIdentifier): Boolean = {
    +    // copied from HiveSessionCatalog
    +    val hiveFunctions = Seq(
    +      "hash",
    +      "histogram_numeric",
    +      "percentile")
    +
    +    // A temporary function is a function that has been registered in functionRegistry
    +    // without a database name, and is neither a built-in function nor a Hive function
    +    name.database.isEmpty &&
    +      functionRegistry.functionExists(name.funcName) &&
    +      !FunctionRegistry.builtin.functionExists(name.funcName) &&
    --- End diff --
    
    Added a test case: https://github.com/gatorsmile/spark/blob/fec0066331aaf085f8cd0318be23844560e72b4b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala#L496


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15764: [SPARK-18217] [SQL] Disallow creating permanent views ba...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15764
  
    **[Test build #68116 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68116/consoleFull)** for PR 15764 at commit [`86e7f9d`](https://github.com/apache/spark/commit/86e7f9d8afdaed72666b79692a31a9131461dc29).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15764: [SPARK-18217] [SQL] Disallow creating permanent v...

Posted by tejasapatil <gi...@git.apache.org>.
Github user tejasapatil commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15764#discussion_r86691637
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala ---
    @@ -923,6 +923,24 @@ class SessionCatalog(
         }
       }
     
    +  /**
    +   * Returns whether it is a temporary function. If not existed, returns false.
    +   */
    +  def isTemporaryFunction(name: FunctionIdentifier): Boolean = {
    +    // copied from HiveSessionCatalog
    +    val hiveFunctions = Seq(
    +      "hash",
    +      "histogram_numeric",
    +      "percentile")
    +
    +    // A temporary function is a function that has been registered in functionRegistry
    +    // without a database name, and is neither a built-in function nor a Hive function
    +    name.database.isEmpty &&
    +      functionRegistry.functionExists(name.funcName) &&
    +      !FunctionRegistry.builtin.functionExists(name.funcName) &&
    +      !hiveFunctions.contains(name.funcName.toLowerCase)
    --- End diff --
    
    I thought `HiveSessionCatalog` is used in context of Hive and `SessionCatalog` has nothing to do with Hive. So if we remove this from here and override `isTemporaryFunction` in `HiveSessionCatalog`, this would look clean. Feels like I am missing something obvious here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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