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

    https://github.com/apache/spark/pull/20377#discussion_r163920410
  
    --- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/IsolatedClientLoader.scala
 ---
    @@ -251,10 +251,8 @@ private[hive] class IsolatedClientLoader(
     
       /** The isolated client interface to Hive. */
       private[hive] def createClient(): HiveClient = synchronized {
    -    val warehouseDir = 
Option(hadoopConf.get(ConfVars.METASTOREWAREHOUSE.varname))
         if (!isolationOn) {
    -      return new HiveClientImpl(version, warehouseDir, sparkConf, 
hadoopConf, config,
    -        baseClassLoader, this)
    +      return new HiveClientImpl(version, sparkConf, hadoopConf, config, 
baseClassLoader, this)
    --- End diff --
    
    Let me ask the question: what exactly is the problem with the argument I 
added? It solves the issue without having to write all this code.
    
    If you really dislike the argument for some odd reason, you can get the 
config by iterating over the `Iterable` in `HiveClientImpl`, making an 
operation that is currently O(1) to be O(n).
    
    But I really don't understand why you guys care about this argument so 
much. There are *2* call sites to that constructor in the whole code base, both 
in the same method in `IsolatedClientLoader.scala`.


---

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

Reply via email to