[GitHub] carbondata pull request #3010: Fix PreAggregate Datamap Issue

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user kumarvishal09 commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user Shubh18s commented on the issue:

    https://github.com/apache/carbondata/pull/3010
 
    Respected Reviewer please Merge this PR.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3010#discussion_r245553484
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -1445,16 +1445,10 @@ private CarbonCommonConstants() {
     
       @CarbonProperty(dynamicConfigurable = true)
       public static final String SUPPORT_DIRECT_QUERY_ON_DATAMAP =
    -      "carbon.query.directQueryOnDataMap.enabled";
    +          "carbon.query.directQueryOnDataMap.enabled";
    --- End diff --
   
    Done


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3010#discussion_r245553499
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateUtil.scala ---
    @@ -626,22 +626,29 @@ object PreAggregateUtil {
         //  schema ordinal should be considered
         columns.sortBy(_.getSchemaOrdinal).foreach { a =>
           if (a.getAggFunction.nonEmpty) {
    -        aggregateColumns += s"${a.getAggFunction match {
    -          case "count" => "sum"
    -          case _ => a.getAggFunction}}(${a.getColumnName})"
    +        aggregateColumns += s"${
    --- End diff --
   
    Done


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3010#discussion_r245553556
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/optimizer/CarbonLateDecodeRule.scala ---
    @@ -106,22 +106,29 @@ class CarbonLateDecodeRule extends Rule[LogicalPlan] with PredicateHelper {
       def validateQueryDirectlyOnDataMap(relations: Seq[CarbonDecoderRelation]): Unit = {
         var isPreAggDataMapExists = false
         // first check if pre aggregate data map exists or not
    -    relations.foreach{relation =>
    +    relations.foreach { relation =>
           if (relation.carbonRelation.carbonTable.isChildDataMap) {
             isPreAggDataMapExists = true
           }
         }
         val validateQuery = CarbonProperties.getInstance
    -      .getProperty(CarbonCommonConstants.VALIDATE_DIRECT_QUERY_ON_DATAMAP,
    -        CarbonCommonConstants.VALIDATE_DIRECT_QUERY_ON_DATAMAP_DEFAULTVALUE).toBoolean
    +      .getProperty(CarbonCommonConstants.VALIDATE_DIRECT_QUERY_ON_DATAMAP)
         var isThrowException = false
         // if validate query is enabled and relation contains pre aggregate data map
    --- End diff --
   
    Done


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3010#discussion_r245553671
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -1449,12 +1449,9 @@ private CarbonCommonConstants() {
     
       public static final String SUPPORT_DIRECT_QUERY_ON_DATAMAP_DEFAULTVALUE = "false";
     
    -  @CarbonProperty
       public static final String VALIDATE_DIRECT_QUERY_ON_DATAMAP =
           "carbon.query.validate.direct.query.on.datamap";
    --- End diff --
   
    Yes, I made changes to configuration-paramters.md accordingly and removed this property.


---

[qiuchenjian-2]

Github user kumarvishal09 commented on the issue:

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


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---