sunchao commented on a change in pull request #33382:
URL: https://github.com/apache/spark/pull/33382#discussion_r671006251



##########
File path: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala
##########
@@ -876,29 +876,21 @@ private[client] class Shim_v0_13 extends Shim_v0_12 {
       } else {
         logDebug(s"Hive metastore filter is '$filter'.")
         val tryDirectSqlConfVar = HiveConf.ConfVars.METASTORE_TRY_DIRECT_SQL
-        // We should get this config value from the metaStore. otherwise hit 
SPARK-18681.
-        // To be compatible with hive-0.12 and hive-0.13, In the future we can 
achieve this by:
-        // val tryDirectSql = 
hive.getMetaConf(tryDirectSqlConfVar.varname).toBoolean
-        val tryDirectSql = 
hive.getMSC.getConfigValue(tryDirectSqlConfVar.varname,
-          tryDirectSqlConfVar.defaultBoolVal.toString).toBoolean
         try {
           // Hive may throw an exception when calling this method in some 
circumstances, such as
           // when filtering on a non-string partition column when the hive 
config key
           // hive.metastore.try.direct.sql is false

Review comment:
       Yeah Hive might throw exception even when it is true, which is what we 
discovered here. The comment, I think, is on the `try ... catch` clause though, 
and it uses the case to explain why we need to catch the exception here. I can 
add a sentence such as "It could also throw exception even if 
`hive.metastore.try.direct.sql` is set to true in certain cases" if that helps 
to understand the code.




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to