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



---

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



---

[qiuchenjian-2]

Github user Shubh18s 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.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2262/



---

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user kunal642 commented on the issue:

    https://github.com/apache/carbondata/pull/3010
 
    @Shubh18s Please fix 2.3 build


---

[qiuchenjian-2]

Github user Shubh18s 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/2107/



---

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



---

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3010#discussion_r244678587
 
    --- 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 --
   
    Revert this change


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3010#discussion_r244678677
 
    --- 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 --
   
    revert this change


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3010#discussion_r244679013
 
    --- 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"${
    +          a.getAggFunction match {
    +            case "count" => "sum"
    +            case _ => a.getAggFunction
    +          }
    +        }(${ a.getColumnName })"
           } else {
             groupingExpressions += a.getColumnName
    -        aggregateColumns+= a.getColumnName
    +        aggregateColumns += a.getColumnName
           }
         }
    +    val groupByString = if (groupingExpressions.nonEmpty) {
    +      s" group by ${ groupingExpressions.mkString(",") }"
    +    } else { "" }
         s"select ${ aggregateColumns.mkString(",") } " +
    -    s"from $databaseName.${ tableSchema.getTableName }" +
    -    s" group by ${ groupingExpressions.mkString(",") }"
    +    s"from $databaseName.${ tableSchema.getTableName }" + groupByString
       }
     
    -  /**
    +  /** CarbonProperties.getInstance().addProperty(CarbonCommonConstants
    --- End diff --
   
    why this change is requried??


---

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



---

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user Shubh18s 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/2136/



---

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



---