You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "yigress (via GitHub)" <gi...@apache.org> on 2023/04/14 22:17:40 UTC

[GitHub] [spark] yigress opened a new pull request, #40799: [SPARK-43145][SQL] Reduce ClassNotFound of hive storage handler table

yigress opened a new pull request, #40799:
URL: https://github.com/apache/spark/pull/40799

   ### What changes were proposed in this pull request?
   where hive table.getStorageHandler call is used, check hive table parameter "storage_handler" first.  purpose is that hive table.getStorageHandler initializes the storagehandler class, if not necessary can just check on hive table parameter first. the table parameter is required for storagehandler table in hive.
   
   
   ### Why are the changes needed?
   for desc table, or use case where user just want to load HiveTableRelation, user do not need to provide the storagehandler jar.
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   
   ### How was this patch tested?
   past unit tests and also local test.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [spark] yigress closed pull request #40799: [SPARK-43145][SQL] Reduce ClassNotFound of hive storage handler table

Posted by "yigress (via GitHub)" <gi...@apache.org>.
yigress closed pull request #40799: [SPARK-43145][SQL] Reduce ClassNotFound of hive storage handler table
URL: https://github.com/apache/spark/pull/40799


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [spark] sunchao commented on a diff in pull request #40799: [SPARK-43145][SQL] Reduce ClassNotFound of hive storage handler table

Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on code in PR #40799:
URL: https://github.com/apache/spark/pull/40799#discussion_r1169247622


##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/TableReader.scala:
##########
@@ -417,6 +417,7 @@ private[hive] object HiveTableUtil {
   def configureJobPropertiesForStorageHandler(
       tableDesc: TableDesc, conf: Configuration, input: Boolean): Unit = {
     val property = tableDesc.getProperties.getProperty(META_TABLE_STORAGE)
+    if (property == null) { return }

Review Comment:
   I don't get this, if `property` is null and we pass to `HiveUtils.getStorageHandler`, wouldn't that simply just return `null` and the rest of this method is skipped?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [spark] yigress commented on a diff in pull request #40799: [SPARK-43145][SQL] Reduce ClassNotFound of hive storage handler table

Posted by "yigress (via GitHub)" <gi...@apache.org>.
yigress commented on code in PR #40799:
URL: https://github.com/apache/spark/pull/40799#discussion_r1169280163


##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/TableReader.scala:
##########
@@ -417,6 +417,7 @@ private[hive] object HiveTableUtil {
   def configureJobPropertiesForStorageHandler(
       tableDesc: TableDesc, conf: Configuration, input: Boolean): Unit = {
     val property = tableDesc.getProperties.getProperty(META_TABLE_STORAGE)
+    if (property == null) { return }

Review Comment:
   @sunchao you are right. here is unnecessary. the original code will return without initializing the storagehandler. i will remove this change.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [spark] yigress commented on pull request #40799: [SPARK-43145][SQL] Reduce ClassNotFound of hive storage handler table

Posted by "yigress (via GitHub)" <gi...@apache.org>.
yigress commented on PR #40799:
URL: https://github.com/apache/spark/pull/40799#issuecomment-1509546979

   @cloud-fan can you help review this? thank you!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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