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

[GitHub] spark pull request: [SPARK-14414] [SQL] improve the error message ...

GitHub user bomeng opened a pull request:

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

    [SPARK-14414] [SQL] improve the error message class hierarchy

    ## What changes were proposed in this pull request?
    
    Before we are using AnalysisException, ParseException, NoSuchFunctionException etc when a parsing error encounters. I am trying to make it consistent and also minimum code impact to the current implementation by changing the class hierarchy. 
    1. NoSuchItemException is removed, it is an abstract class and it just simply takes a message string.
    2. NoSuchDatabaseException, NoSuchTableException, NoSuchPartitionException and NoSuchFunctionException now extends AnalysisException, as well as ParseException, they are all under AnalysisException umbrella, but you can also determine how to use them in a granular way.
    
    ## How was this patch tested?
    The existing test cases should cover this patch.


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

    $ git pull https://github.com/bomeng/spark SPARK-14414

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

    https://github.com/apache/spark/pull/12314.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 #12314
    
----
commit ee3c4f72d83cb00d3debefde96cfc050321eddb2
Author: bomeng <bm...@us.ibm.com>
Date:   2016-04-12T00:27:34Z

    improve the error message class hierarchy

----


---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#issuecomment-208703320
  
    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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#discussion_r59468158
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala ---
    @@ -17,36 +17,21 @@
     
     package org.apache.spark.sql.catalyst.analysis
     
    +import org.apache.spark.sql.AnalysisException
     import org.apache.spark.sql.catalyst.catalog.ExternalCatalog.TablePartitionSpec
     
     
     /**
      * Thrown by a catalog when an item cannot be found. The analyzer will rethrow the exception
      * as an [[org.apache.spark.sql.AnalysisException]] with the correct position information.
      */
    -abstract class NoSuchItemException extends Exception {
    -  override def getMessage: String
    -}
    +class NoSuchDatabaseException(db: String) extends AnalysisException(s"Database $db not found")
     
    -class NoSuchDatabaseException(db: String) extends NoSuchItemException {
    -  override def getMessage: String = s"Database $db not found"
    -}
    +class NoSuchTableException(db: String, table: String)
    +  extends AnalysisException(s"Table or View $table not found in database $db")
     
    -class NoSuchTableException(db: String, table: String) extends NoSuchItemException {
    -  override def getMessage: String = s"Table or View $table not found in database $db"
    -}
    +class NoSuchPartitionException(db: String, table: String, spec: TablePartitionSpec) extends
    +    AnalysisException(s"Partition not found in table $table database $db:\n" + spec.mkString("\n"))
    --- End diff --
    
    It will become due to line limit:
    `class NoSuchPartitionException(
        db: String,
        table: String,
        spec: TablePartitionSpec)
      extends
        AnalysisException(s"Partition not found in table $table database $db:\n" + spec.mkString("\n"))`


---
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: [SPARK-14414] [SQL] improve the error message ...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/12314#issuecomment-209115133
  
    There's a lot more to this issue than just changing this hierarchy. In the parsers we still throw `AnalysisException` sometimes but `ParseException` others. I'm re-opening this.


---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#issuecomment-209084525
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55629/
    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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#discussion_r59459425
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala ---
    @@ -17,36 +17,21 @@
     
     package org.apache.spark.sql.catalyst.analysis
     
    +import org.apache.spark.sql.AnalysisException
     import org.apache.spark.sql.catalyst.catalog.ExternalCatalog.TablePartitionSpec
     
     
     /**
      * Thrown by a catalog when an item cannot be found. The analyzer will rethrow the exception
      * as an [[org.apache.spark.sql.AnalysisException]] with the correct position information.
      */
    -abstract class NoSuchItemException extends Exception {
    -  override def getMessage: String
    -}
    +class NoSuchDatabaseException(db: String) extends AnalysisException(s"Database $db not found")
     
    -class NoSuchDatabaseException(db: String) extends NoSuchItemException {
    -  override def getMessage: String = s"Database $db not found"
    -}
    +class NoSuchTableException(db: String, table: String)
    +  extends AnalysisException(s"Table or View $table not found in database $db")
     
    -class NoSuchTableException(db: String, table: String) extends NoSuchItemException {
    -  override def getMessage: String = s"Table or View $table not found in database $db"
    -}
    +class NoSuchPartitionException(db: String, table: String, spec: TablePartitionSpec) extends
    +    AnalysisException(s"Partition not found in table $table database $db:\n" + spec.mkString("\n"))
    --- End diff --
    
    style:
    ```
    class NoSuch...Exception(
        db: String,
        table: String,
        spec: ...)
      extends AnalysisException("...")
    ```


---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#issuecomment-208671172
  
    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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#issuecomment-208671175
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55560/
    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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#discussion_r59606712
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala ---
    @@ -17,36 +17,21 @@
     
     package org.apache.spark.sql.catalyst.analysis
     
    +import org.apache.spark.sql.AnalysisException
     import org.apache.spark.sql.catalyst.catalog.ExternalCatalog.TablePartitionSpec
     
     
     /**
      * Thrown by a catalog when an item cannot be found. The analyzer will rethrow the exception
      * as an [[org.apache.spark.sql.AnalysisException]] with the correct position information.
      */
    -abstract class NoSuchItemException extends Exception {
    -  override def getMessage: String
    -}
    +class NoSuchDatabaseException(db: String) extends AnalysisException(s"Database $db not found")
     
    -class NoSuchDatabaseException(db: String) extends NoSuchItemException {
    -  override def getMessage: String = s"Database $db not found"
    -}
    +class NoSuchTableException(db: String, table: String)
    +  extends AnalysisException(s"Table or View $table not found in database $db")
     
    -class NoSuchTableException(db: String, table: String) extends NoSuchItemException {
    -  override def getMessage: String = s"Table or View $table not found in database $db"
    -}
    +class NoSuchPartitionException(db: String, table: String, spec: TablePartitionSpec) extends
    +    AnalysisException(s"Partition not found in table $table database $db:\n" + spec.mkString("\n"))
    --- End diff --
    
    it's OK I'll fix these later.


---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#issuecomment-208684019
  
    **[Test build #55577 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55577/consoleFull)** for PR 12314 at commit [`b12229f`](https://github.com/apache/spark/commit/b12229ff1be7925f230a0de2c17243b2c27814e6).


---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#issuecomment-208703324
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55577/
    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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#issuecomment-208642128
  
    **[Test build #55560 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55560/consoleFull)** for PR 12314 at commit [`ee3c4f7`](https://github.com/apache/spark/commit/ee3c4f72d83cb00d3debefde96cfc050321eddb2).


---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#discussion_r59314286
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
    @@ -66,8 +65,7 @@ private[spark] class HiveExternalCatalog(client: HiveClient) extends ExternalCat
         try {
           body
         } catch {
    -      case e: NoSuchItemException =>
    -        throw new AnalysisException(e.getMessage)
    +      case e: AnalysisException => throw new AnalysisException(e.getMessage)
    --- End diff --
    
    I am open to either way. The original code is to catch `NoSuchItemException` and throw `AnalysisException`, which I did not want to change any logic in this patch. 


---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#issuecomment-208703085
  
    **[Test build #55577 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55577/consoleFull)** for PR 12314 at commit [`b12229f`](https://github.com/apache/spark/commit/b12229ff1be7925f230a0de2c17243b2c27814e6).
     * 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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#discussion_r59313257
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
    @@ -66,8 +65,7 @@ private[spark] class HiveExternalCatalog(client: HiveClient) extends ExternalCat
         try {
           body
         } catch {
    -      case e: NoSuchItemException =>
    -        throw new AnalysisException(e.getMessage)
    +      case e: AnalysisException => throw new AnalysisException(e.getMessage)
    --- End diff --
    
    The effect of this is to lose the original stack trace and replace it with a new one here. Is that desirable vs just removing this line to let the original exception propagate?


---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#discussion_r59315189
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
    @@ -66,8 +65,7 @@ private[spark] class HiveExternalCatalog(client: HiveClient) extends ExternalCat
         try {
           body
         } catch {
    -      case e: NoSuchItemException =>
    -        throw new AnalysisException(e.getMessage)
    +      case e: AnalysisException => throw new AnalysisException(e.getMessage)
    --- End diff --
    
    @srowen I will make it change shortly by removing this line. 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: [SPARK-14414] [SQL] improve the error message ...

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

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


---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#issuecomment-208658715
  
    LGTM pending tests


---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#issuecomment-209084518
  
    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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#discussion_r59466520
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
    @@ -29,6 +29,7 @@ import org.apache.spark.sql.catalyst.expressions.{Attribute, AttributeReference}
     import org.apache.spark.sql.types._
     
     
    +
    --- End diff --
    
    ParseException is a subclass of AnalysisException, which I think it is fine with my new class hierarchy. Do you want me still work on this for changing the coding style? 


---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#issuecomment-208670574
  
    **[Test build #55560 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55560/consoleFull)** for PR 12314 at commit [`ee3c4f7`](https://github.com/apache/spark/commit/ee3c4f72d83cb00d3debefde96cfc050321eddb2).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class NoSuchDatabaseException(db: String) extends AnalysisException(s\"Database $db not found\")`
      * `class NoSuchTableException(db: String, table: String)`
      * `class NoSuchFunctionException(db: String, func: String)`


---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#discussion_r59606692
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala ---
    @@ -17,36 +17,21 @@
     
     package org.apache.spark.sql.catalyst.analysis
     
    +import org.apache.spark.sql.AnalysisException
     import org.apache.spark.sql.catalyst.catalog.ExternalCatalog.TablePartitionSpec
     
     
     /**
      * Thrown by a catalog when an item cannot be found. The analyzer will rethrow the exception
      * as an [[org.apache.spark.sql.AnalysisException]] with the correct position information.
      */
    -abstract class NoSuchItemException extends Exception {
    -  override def getMessage: String
    -}
    +class NoSuchDatabaseException(db: String) extends AnalysisException(s"Database $db not found")
     
    -class NoSuchDatabaseException(db: String) extends NoSuchItemException {
    -  override def getMessage: String = s"Database $db not found"
    -}
    +class NoSuchTableException(db: String, table: String)
    +  extends AnalysisException(s"Table or View $table not found in database $db")
     
    -class NoSuchTableException(db: String, table: String) extends NoSuchItemException {
    -  override def getMessage: String = s"Table or View $table not found in database $db"
    -}
    +class NoSuchPartitionException(db: String, table: String, spec: TablePartitionSpec) extends
    +    AnalysisException(s"Partition not found in table $table database $db:\n" + spec.mkString("\n"))
    --- End diff --
    
    Then I would do:
    ```
    class NoSuch...Exception(
        db: String,
        table: String,
        spec: ...)
      extends AnalysisException(
        "...")
    ```


---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#issuecomment-208724448
  
    @bomeng  this no longer merges cleanly. Can you rebase?



---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#issuecomment-209042694
  
    **[Test build #55629 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55629/consoleFull)** for PR 12314 at commit [`8ec5069`](https://github.com/apache/spark/commit/8ec506980dcbb6f8f6eee7db3fe31fb6085eaded).


---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#discussion_r59314537
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala ---
    @@ -66,8 +65,7 @@ private[spark] class HiveExternalCatalog(client: HiveClient) extends ExternalCat
         try {
           body
         } catch {
    -      case e: NoSuchItemException =>
    -        throw new AnalysisException(e.getMessage)
    +      case e: AnalysisException => throw new AnalysisException(e.getMessage)
    --- End diff --
    
    It would have had to to throw `AnalysisException`. I suspect there's no reason now to lose the original stack trace.


---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#issuecomment-209094374
  
    Thanks - merging in master.



---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#discussion_r59459447
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
    @@ -29,6 +29,7 @@ import org.apache.spark.sql.catalyst.expressions.{Attribute, AttributeReference}
     import org.apache.spark.sql.types._
     
     
    +
    --- End diff --
    
    don't add this


---
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: [SPARK-14414] [SQL] improve the error message ...

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

    https://github.com/apache/spark/pull/12314#issuecomment-209084172
  
    **[Test build #55629 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55629/consoleFull)** for PR 12314 at commit [`8ec5069`](https://github.com/apache/spark/commit/8ec506980dcbb6f8f6eee7db3fe31fb6085eaded).
     * 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