[GitHub] carbondata pull request #1575: [CARBONDATA-1698]Adding support for table lev...

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1575
 
    Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/953/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1575#discussion_r158054020
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/datacompaction/TableLevelCompactionParameterTest.scala ---
    @@ -0,0 +1,183 @@
    +/*
    --- End diff --
   
    This test case is same as TableLevelCompactionOptionTest.scala, please remove this one(TableLevelCompactionParameterTest.scala)


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1575#discussion_r158056650
 
    --- Diff: integration/spark2/src/main/scala/org/apache/carbondata/spark/rdd/CarbonDataRDDFactory.scala ---
    @@ -205,7 +205,8 @@ object CarbonDataRDDFactory {
     
                   val newCarbonLoadModel = prepareCarbonLoadModel(table)
     
    -              val compactionSize = CarbonDataMergerUtil.getCompactionSize(CompactionType.MAJOR)
    +              val compactionSize = CarbonDataMergerUtil
    +                .getCompactionSize(CompactionType.MAJOR, carbonLoadModel)
    --- End diff --
   
    why need add one more input parameter(carbonLoadModel) for getCompactionSize()?


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1575#discussion_r158186274
 
    --- Diff: integration/spark2/src/main/scala/org/apache/carbondata/spark/rdd/CarbonDataRDDFactory.scala ---
    @@ -205,7 +205,8 @@ object CarbonDataRDDFactory {
     
                   val newCarbonLoadModel = prepareCarbonLoadModel(table)
     
    -              val compactionSize = CarbonDataMergerUtil.getCompactionSize(CompactionType.MAJOR)
    +              val compactionSize = CarbonDataMergerUtil
    +                .getCompactionSize(CompactionType.MAJOR, carbonLoadModel)
    --- End diff --
   
    carbonLoadModel may contain table-level major compaction size if it is specified in create table SQL, so the purpose for adding parameter 'carbonLoadModel' is to get the table-level major compaction size.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1575#discussion_r158188263
 
    --- Diff: integration/spark2/src/main/scala/org/apache/carbondata/spark/rdd/CarbonDataRDDFactory.scala ---
    @@ -205,7 +205,8 @@ object CarbonDataRDDFactory {
     
                   val newCarbonLoadModel = prepareCarbonLoadModel(table)
     
    -              val compactionSize = CarbonDataMergerUtil.getCompactionSize(CompactionType.MAJOR)
    +              val compactionSize = CarbonDataMergerUtil
    +                .getCompactionSize(CompactionType.MAJOR, carbonLoadModel)
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user Xaprice commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1575
 
    Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/978/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1575
 
    Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/986/



---

[qiuchenjian-2]

Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/1575
 
    LGTM, good contribution.


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---