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

    https://github.com/apache/spark/pull/23268#discussion_r240101927
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveShim.scala 
---
    @@ -53,19 +53,12 @@ private[hive] object HiveShim {
        * This function in hive-0.13 become private, but we have to do this to 
work around hive bug
        */
       private def appendReadColumnNames(conf: Configuration, cols: 
Seq[String]) {
    -    val old: String = 
conf.get(ColumnProjectionUtils.READ_COLUMN_NAMES_CONF_STR, "")
    -    val result: StringBuilder = new StringBuilder(old)
    -    var first: Boolean = old.isEmpty
    -
    -    for (col <- cols) {
    -      if (first) {
    -        first = false
    -      } else {
    -        result.append(',')
    -      }
    -      result.append(col)
    -    }
    -    conf.set(ColumnProjectionUtils.READ_COLUMN_NAMES_CONF_STR, 
result.toString)
    +    val key = ColumnProjectionUtils.READ_COLUMN_NAMES_CONF_STR
    +    val value = Option(conf.get(key, null))
    +      .map(old => cols.+:(old))
    +      .getOrElse(cols)
    +      .mkString(",")
    --- End diff --
    
    Right, but I don't think it's more readable. It's non-critical path so 
performance concern is secondary anyway.


---

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

Reply via email to