[GitHub] carbondata pull request #3032: [CARBONDATA-3210] merge getKeyOnPrefix into C...

[qiuchenjian-2]

Github user xiaohui0318 commented on the issue:

    https://github.com/apache/carbondata/pull/3032
 
    > Be sure to do all of the following checklist to help us incorporate
    > your contribution quickly and easily:
    >
    > * [x]  Any interfaces changed?
    NO
    > * [x]  Any backward compatibility impacted?
    NO
    > * [x]  Document update required?
    NO
    > * [x]  Testing done
    NO,only fix style error
    > * [x]  For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
    NO
   



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3032
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2088/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3032
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2090/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3032
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2295/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3032
 
    Build Failed  with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10344/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3032
 
    Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10345/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3032#discussion_r244596067
 
    --- Diff: integration/spark2/src/main/scala/org/apache/carbondata/spark/util/CarbonSparkUtil.scala ---
    @@ -28,11 +29,15 @@ import org.apache.carbondata.core.metadata.schema.table.{CarbonTable, TableInfo}
     import org.apache.carbondata.core.metadata.schema.table.column.{CarbonColumn, ColumnSchema}
     import org.apache.carbondata.core.util.CarbonUtil
     
    -case class TransformHolder(rdd: Any, mataData: CarbonMetaData)
    +case class TransformHolder (rdd: Any, mataData: CarbonMetaData)
    --- End diff --
   
    remove blank before '('


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3032#discussion_r244596094
 
    --- Diff: integration/spark2/src/main/scala/org/apache/carbondata/spark/util/CarbonSparkUtil.scala ---
    @@ -28,11 +29,15 @@ import org.apache.carbondata.core.metadata.schema.table.{CarbonTable, TableInfo}
     import org.apache.carbondata.core.metadata.schema.table.column.{CarbonColumn, ColumnSchema}
     import org.apache.carbondata.core.util.CarbonUtil
     
    -case class TransformHolder(rdd: Any, mataData: CarbonMetaData)
    +case class TransformHolder (rdd: Any, mataData: CarbonMetaData)
    +
    + /**
    +  * carbon spark common methods
    +  */
     
     object CarbonSparkUtil {
     
    -  def createSparkMeta(carbonTable: CarbonTable): CarbonMetaData = {
    +  def createSparkMeta (carbonTable: CarbonTable): CarbonMetaData = {
    --- End diff --
   
    remove blank before '('


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3032#discussion_r244596100
 
    --- Diff: integration/spark2/src/main/scala/org/apache/carbondata/spark/util/CarbonSparkUtil.scala ---
    @@ -50,7 +55,7 @@ object CarbonSparkUtil {
           CarbonUtil.hasAggregationDataMap(carbonTable))
       }
     
    -  def createCarbonRelation(tableInfo: TableInfo, tablePath: String): CarbonRelation = {
    +  def createCarbonRelation (tableInfo: TableInfo, tablePath: String): CarbonRelation = {
    --- End diff --
   
    remove blank before '('


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3032#discussion_r244596104
 
    --- Diff: integration/spark2/src/main/scala/org/apache/carbondata/spark/util/CarbonSparkUtil.scala ---
    @@ -59,13 +64,13 @@ object CarbonSparkUtil {
           table)
       }
     
    -  /**
    -   * return's the formatted column comment if column comment is present else empty("")
    -   *
    -   * @param carbonColumn
    -   * @return
    -   */
    -  def getColumnComment(carbonColumn: CarbonColumn): String = {
    +   /**
    +    * return's the formatted column comment if column comment is present else empty("")
    +    *
    +    * @param carbonColumn the column of carbonTable
    +    * @return string
    +    */
    +  def getColumnComment (carbonColumn: CarbonColumn): String = {
    --- End diff --
   
    remove blank before '('


---

[qiuchenjian-2]

Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/3032
 
    please remove blank before '('


---

[qiuchenjian-2]

Github user xiaohui0318 commented on the issue:

    https://github.com/apache/carbondata/pull/3032
 
    fix already.tks


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3032
 
    Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10352/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3032#discussion_r245324728
 
    --- Diff: integration/spark2/src/main/scala/org/apache/carbondata/spark/util/CarbonSparkUtil.scala ---
    @@ -30,6 +31,10 @@ import org.apache.carbondata.core.util.CarbonUtil
     
     case class TransformHolder(rdd: Any, mataData: CarbonMetaData)
     
    + /**
    +  * carbon spark common methods
    +  */
    +
    --- End diff --
   
    remove this blank line


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3032#discussion_r245324988
 
    --- Diff: integration/spark2/src/main/scala/org/apache/carbondata/spark/util/CarbonSparkUtil.scala ---
    @@ -108,13 +113,45 @@ object CarbonSparkUtil {
         fields.mkString(",")
       }
     
    -  /**
    -   * add escape prefix for delimiter
    -   */
    +   /**
    +    * add escape prefix for delimiter
    +    *
    +    * @param delimiter A delimiter is a sequence of one or more characters
    +    * used to specify the boundary between separate
    +    * @return delimiter
    +    */
       def delimiterConverter4Udf(delimiter: String): String = delimiter match {
         case "|" | "*" | "." | ":" | "^" | "\\" | "$" | "+" | "?" | "(" | ")" | "{" | "}" | "[" | "]" =>
           "\\\\" + delimiter
         case _ =>
           delimiter
       }
    +
    +
    --- End diff --
   
    remove


---

[qiuchenjian-2]

Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/3032
 
    Just some nit comments, and others LGTM


---