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

[qiuchenjian-2]

GitHub user dhatchayani opened a pull request:

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

    [CARBONDATA-1706] Making index merge DDL insensitive of the property

    (1) Segment index merge DDL will be insensitive to the property. Even if the property is set to false, DDL should be able to merge index files
    (2) Compaction name is changed from **SEGMENT_INDEX_COMPACTION** to **SEGMENT_INDEX**
   
     - [ ] Any interfaces changed?
     
     - [ ] Any backward compatibility impacted?
     
     - [X] Document update required?
             Compaction name change should be updated in document accordingly
   
     - [X] Testing done
            UT Added
           
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
   


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/dhatchayani/incubator-carbondata merge_index

Alternatively you can review and apply these changes as the patch at:

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

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1494
   
----
commit 5bfd54c6b392b85d3910de6c44afe78c4248057d
Author: dhatchayani <[hidden email]>
Date:   2017-11-14T09:47:14Z

    [CARBONDATA-1706] Making index merge DDL insensitive of the 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/1109/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1494
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1708/



---

[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/1118/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1494#discussion_r150854506
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/management/AlterTableCompactionCommand.scala ---
    @@ -128,11 +127,18 @@ case class AlterTableCompactionCommand(
           CommonUtil.readLoadMetadataDetails(carbonLoadModel)
         }
         if (compactionType == CompactionType.SEGMENT_INDEX_COMPACTION) {
    +      val mergeIndexProperty = CarbonProperties.getInstance()
    +        .getProperty(CarbonCommonConstants.CARBON_MERGE_INDEX_IN_SEGMENT)
    +      CarbonProperties.getInstance()
    --- End diff --
   
    Better pass the boolean to `CommonUtil.mergeIndexFiles` rather than setting to `CarbonProperties`


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1494
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1718/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[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/1142/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1494
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1729/



---

[qiuchenjian-2]

Github user dhatchayani commented on the issue:

    https://github.com/apache/carbondata/pull/1494
 
    retest this please


---

[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/1151/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1494#discussion_r151131189
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/spark/util/CommonUtil.scala ---
    @@ -851,13 +851,21 @@ 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) {
    -      new CarbonMergeFilesRDD(sparkContext, AbsoluteTableIdentifier.from(tablePath,
    -        carbonTable.getDatabaseName, carbonTable.getFactTableName).getTablePath,
    -        segmentIds).collect()
    +      carbonTable: CarbonTable,
    +      mergeIndexProperty: Option[Boolean]): Unit = {
    +    mergeIndexProperty match {
    +      case Some(true) =>
    +        new CarbonMergeFilesRDD(sparkContext, AbsoluteTableIdentifier.from(tablePath,
    +          carbonTable.getDatabaseName, carbonTable.getFactTableName).getTablePath,
    +          segmentIds).collect()
    +      case _ =>
    +        if (CarbonProperties.getInstance().getProperty(
    +          CarbonCommonConstants.CARBON_MERGE_INDEX_IN_SEGMENT,
    +          CarbonCommonConstants.CARBON_MERGE_INDEX_IN_SEGMENT_DEFAULT).toBoolean) {
    --- End diff --
   
    Add the validation for boolean in case user pass wrong boolean parameter then take default


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1494
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1741/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user dhatchayani commented on the issue:

    https://github.com/apache/carbondata/pull/1494
 
    retest this please



---

[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/1166/



---

[qiuchenjian-2]

Github user dhatchayani commented on the issue:

    https://github.com/apache/carbondata/pull/1494
 
    retest this please


---

[qiuchenjian-2]

Github user dhatchayani commented on the issue:

    https://github.com/apache/carbondata/pull/1494
 
    retest sdv please


---

[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/1170/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1494
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1750/



---