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

[GitHub] spark pull request: [SPARK-14869][SQL] Don't mask exceptions in Re...

GitHub user rxin opened a pull request:

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

    [SPARK-14869][SQL] Don't mask exceptions in ResolveRelations

    ## What changes were proposed in this pull request?
    In order to support SPARK-11197 (run SQL directly on files), we added some code in ResolveRelations to catch the exception thrown by catalog.lookupRelation and ignore it. This unfortunately masks all the exceptions. It should've been sufficient to simply test the table does not exist.
    
    ## How was this patch tested?
    I manually hacked some bugs into Spark and made sure the exceptions are being propagated up.
    


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

    $ git pull https://github.com/rxin/spark SPARK-14869

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

    https://github.com/apache/spark/pull/12634.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 #12634
    
----
commit 43cc738be656fa8978da3970ce3224b5a4f0c1a3
Author: Reynold Xin <rx...@databricks.com>
Date:   2016-04-23T06:59:36Z

    [SPARK-14869][SQL] Don't mask exceptions in ResolveRelations

----


---
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-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#issuecomment-213817224
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/56805/
    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-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#discussion_r60848581
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -412,20 +412,26 @@ class Analyzer(
             catalog.lookupRelation(u.tableIdentifier, u.alias)
           } catch {
             case _: NoSuchTableException =>
    -          u.failAnalysis(s"Table or View not found: ${u.tableName}")
    +          u.failAnalysis(s"Table or view not found: ${u.tableName}")
           }
         }
     
         def apply(plan: LogicalPlan): LogicalPlan = plan resolveOperators {
           case i @ InsertIntoTable(u: UnresolvedRelation, _, _, _, _) =>
             i.copy(table = EliminateSubqueryAliases(lookupTableFromCatalog(u)))
           case u: UnresolvedRelation =>
    -        try {
    +        val table = u.tableIdentifier
    +        if (table.database.isDefined && conf.runSQLonFile &&
    +            (!catalog.databaseExists(table.database.get) || !catalog.tableExists(table))) {
    +          // If the table does not exist, and the database part is specified, and we support
    +          // running SQL directly on files, then let's just return the original UnresolvedRelation.
    +          // It is possible we are matching a query like "select * from parquet.`/path/to/query`".
    +          // The plan will get resolved later.
    +          // Note that we are testing (!db_exists || !table_exists) because the catalog throws
    --- End diff --
    
    We can add it when it shows up more. So far this is the only place.
    
    On Sunday, April 24, 2016, tedyu <no...@github.com> wrote:
    
    > In
    > sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
    > <https://github.com/apache/spark/pull/12634#discussion_r60845292>:
    >
    > >        }
    > >      }
    > >
    > >      def apply(plan: LogicalPlan): LogicalPlan = plan resolveOperators {
    > >        case i @ InsertIntoTable(u: UnresolvedRelation, _, _, _, _) =>
    > >          i.copy(table = EliminateSubqueryAliases(lookupTableFromCatalog(u)))
    > >        case u: UnresolvedRelation =>
    > > -        try {
    > > +        val table = u.tableIdentifier
    > > +        if (table.database.isDefined && conf.runSQLonFile &&
    > > +            (!catalog.databaseExists(table.database.get) || !catalog.tableExists(table))) {
    > > +          // If the table does not exist, and the database part is specified, and we support
    > > +          // running SQL directly on files, then let's just return the original UnresolvedRelation.
    > > +          // It is possible we are matching a query like "select * from parquet.`/path/to/query`".
    > > +          // The plan will get resolved later.
    > > +          // Note that we are testing (!db_exists || !table_exists) because the catalog throws
    >
    > Maybe add a helper method in catalog with database and table name which
    > checks existence of database first, followed by existence of table.
    >
    > —
    > You are receiving this because you authored the thread.
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/spark/pull/12634/files/cddc66b62d89bb35c3a53d307d9d3410fe133e16#r60845292>
    >



---
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-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#issuecomment-213694868
  
    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: [SPARK-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#issuecomment-213678447
  
    LGTM


---
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-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#issuecomment-213694843
  
    **[Test build #56789 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56789/consoleFull)** for PR 12634 at commit [`a1980dc`](https://github.com/apache/spark/commit/a1980dccc8963e1ae0e15385d8bf8b79a5d5fdec).
     * 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 pull request: [SPARK-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#issuecomment-213797489
  
    **[Test build #56805 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56805/consoleFull)** for PR 12634 at commit [`cddc66b`](https://github.com/apache/spark/commit/cddc66b62d89bb35c3a53d307d9d3410fe133e16).


---
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-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#discussion_r60832345
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/CatalystConf.scala ---
    @@ -32,6 +32,8 @@ trait CatalystConf {
       def optimizerInSetConversionThreshold: Int
       def maxCaseBranchesForCodegen: Int
     
    +  def runSQLonFile: Boolean
    --- End diff --
    
    we have never formalized this, but the java convention I believe is for acronyms larger than 4 letters, use Pascal case. For acronyms shorter than 4, upper is prefered.


---
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-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#issuecomment-213694869
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/56789/
    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: [SPARK-14869][SQL] Don't mask exceptions in Re...

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

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


---
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-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#discussion_r60845292
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
    @@ -412,20 +412,26 @@ class Analyzer(
             catalog.lookupRelation(u.tableIdentifier, u.alias)
           } catch {
             case _: NoSuchTableException =>
    -          u.failAnalysis(s"Table or View not found: ${u.tableName}")
    +          u.failAnalysis(s"Table or view not found: ${u.tableName}")
           }
         }
     
         def apply(plan: LogicalPlan): LogicalPlan = plan resolveOperators {
           case i @ InsertIntoTable(u: UnresolvedRelation, _, _, _, _) =>
             i.copy(table = EliminateSubqueryAliases(lookupTableFromCatalog(u)))
           case u: UnresolvedRelation =>
    -        try {
    +        val table = u.tableIdentifier
    +        if (table.database.isDefined && conf.runSQLonFile &&
    +            (!catalog.databaseExists(table.database.get) || !catalog.tableExists(table))) {
    +          // If the table does not exist, and the database part is specified, and we support
    +          // running SQL directly on files, then let's just return the original UnresolvedRelation.
    +          // It is possible we are matching a query like "select * from parquet.`/path/to/query`".
    +          // The plan will get resolved later.
    +          // Note that we are testing (!db_exists || !table_exists) because the catalog throws
    --- End diff --
    
    Maybe add a helper method in catalog with database and table name which checks existence of database first, followed by existence of table.



---
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-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#issuecomment-213677747
  
    cc @davies / @andrewor14 


---
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-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#issuecomment-213817154
  
    **[Test build #56805 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56805/consoleFull)** for PR 12634 at commit [`cddc66b`](https://github.com/apache/spark/commit/cddc66b62d89bb35c3a53d307d9d3410fe133e16).
     * 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-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#issuecomment-213707877
  
    LGTM


---
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-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#issuecomment-213818904
  
    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-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#issuecomment-213817223
  
    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-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#issuecomment-213679008
  
    **[Test build #56787 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56787/consoleFull)** for PR 12634 at commit [`43cc738`](https://github.com/apache/spark/commit/43cc738be656fa8978da3970ce3224b5a4f0c1a3).
     * 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 pull request: [SPARK-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#issuecomment-213678085
  
    **[Test build #56787 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56787/consoleFull)** for PR 12634 at commit [`43cc738`](https://github.com/apache/spark/commit/43cc738be656fa8978da3970ce3224b5a4f0c1a3).


---
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-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#issuecomment-213679032
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/56787/
    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: [SPARK-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#issuecomment-213679029
  
    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: [SPARK-14869][SQL] Don't mask exceptions in Re...

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

    https://github.com/apache/spark/pull/12634#issuecomment-213681559
  
    **[Test build #56789 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56789/consoleFull)** for PR 12634 at commit [`a1980dc`](https://github.com/apache/spark/commit/a1980dccc8963e1ae0e15385d8bf8b79a5d5fdec).


---
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-14869][SQL] Don't mask exceptions in Re...

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/12634#discussion_r60826409
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/CatalystConf.scala ---
    @@ -32,6 +32,8 @@ trait CatalystConf {
       def optimizerInSetConversionThreshold: Int
       def maxCaseBranchesForCodegen: Int
     
    +  def runSQLonFile: Boolean
    --- End diff --
    
    nit: should be `runSqlOnFile`?


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