[GitHub] carbondata pull request #1494: [CARBONDATA-1706] Making index merge DDL inse...

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1494#discussion_r151395517
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/spark/rdd/Compactor.scala ---
    @@ -113,7 +113,8 @@ object Compactor {
     
         if (finalMergeStatus) {
           val mergedLoadNumber = CarbonDataMergerUtil.getLoadNumberFromLoadName(mergedLoadName)
    -      CommonUtil.mergeIndexFiles(sc.sparkContext, Seq(mergedLoadNumber), storePath, carbonTable)
    +      CommonUtil
    +        .mergeIndexFiles(sc.sparkContext, Seq(mergedLoadNumber), storePath, carbonTable, false)
    --- End diff --
   
    move line up and break from parameters like
    ```
    CommonUtil.mergeIndexFiles(
         sc.sparkContext, Seq(mergedLoadNumber), storePath, carbonTable, false)
    ```


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1494#discussion_r151395683
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/spark/util/CommonUtil.scala ---
    @@ -851,13 +851,20 @@ object CommonUtil {
       def mergeIndexFiles(sparkContext: SparkContext,
           segmentIds: Seq[String],
           tablePath: String,
    -      carbonTable: CarbonTable): Unit = {
    -    if (CarbonProperties.getInstance().getProperty(
    -      CarbonCommonConstants.CARBON_MERGE_INDEX_IN_SEGMENT,
    -      CarbonCommonConstants.CARBON_MERGE_INDEX_IN_SEGMENT_DEFAULT).toBoolean) {
    +      carbonTable: CarbonTable,
    +      mergeIndexProperty: Boolean): Unit = {
    +    if (mergeIndexProperty) {
           new CarbonMergeFilesRDD(sparkContext, AbsoluteTableIdentifier.from(tablePath,
             carbonTable.getDatabaseName, carbonTable.getFactTableName).getTablePath,
             segmentIds).collect()
    +    } else {
    +      if (CarbonProperties.getInstance().getProperty(
    +        CarbonCommonConstants.CARBON_MERGE_INDEX_IN_SEGMENT,
    +        CarbonCommonConstants.CARBON_MERGE_INDEX_IN_SEGMENT_DEFAULT).toBoolean) {
    --- End diff --
   
    what if the property passed by user is wrong? can we use default if user passes wrong property


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1494
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1188/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1494
 
    LGTM


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

    https://github.com/apache/carbondata/pull/1494


---