[GitHub] carbondata pull request #2791: [HOTFIX]correct the exception handling in loo...

[qiuchenjian-2]

Github user akashrn5 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2791#discussion_r222537366
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonFileMetastore.scala ---
    @@ -208,7 +209,10 @@ class CarbonFileMetastore extends CarbonMetaStore {
         try {
           lookupRelation(tableIdentifier)(sparkSession)
         } catch {
    -      case _: Exception =>
    +      case ex: Exception =>
    +        if (ex.getCause.isInstanceOf[HiveException]) {
    +          throw ex
    +        }
    --- End diff --
   
    here we arereturning false directly after catching exception, telling the table does not exists, this is wrong, i have check the flow , we might get hiveException(regarding permission) also if the user is not allowed to access the table, that time also we return false and we will not get the proper error.


---

[qiuchenjian-2]

Github user akashrn5 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2791#discussion_r222537606
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonFileMetastore.scala ---
    @@ -23,6 +23,7 @@ import java.net.URI
     import scala.collection.mutable.ArrayBuffer
     
     import org.apache.hadoop.fs.permission.{FsAction, FsPermission}
    +import org.apache.hadoop.hive.ql.metadata.HiveException
    --- End diff --
   
    scalastyle fails if i give the space, i have checked it



---

[qiuchenjian-2]

Github user KanakaKumar commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2791#discussion_r222594726
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonFileMetastore.scala ---
    @@ -208,7 +209,10 @@ class CarbonFileMetastore extends CarbonMetaStore {
         try {
           lookupRelation(tableIdentifier)(sparkSession)
         } catch {
    -      case _: Exception =>
    +      case ex: Exception =>
    +        if (ex.getCause.isInstanceOf[HiveException]) {
    +          throw ex
    +        }
    --- End diff --
   
    There are exceptions from different hierarchy incase of table not found, not all are HiveException.
    Ex:- NoSuchTableException extends  AnalysisException, InvalidTableException extends HiveException.
    So, I think we can refer the exceptions from Spark Code related to table not exists, not found and return false for only those exception cases. All other exceptions we can throw back to caller.


---

[qiuchenjian-2]

Github user akashrn5 commented on the issue:

    https://github.com/apache/carbondata/pull/2791
 
    i am marking it as WIP, i will check and whitelost all the other exception which can be thrown from spark


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2791
 
    Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1055/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2791
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/933/



---

[qiuchenjian-2]

Github user akashrn5 closed the pull request at:

    https://github.com/apache/carbondata/pull/2791


---