[GitHub] carbondata pull request #2478: [CARBONDATA-2540][CARBONDATA-2560][CARBONDATA...

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2478#discussion_r204479261
 
    --- Diff: datamap/mv/core/src/main/scala/org/apache/carbondata/mv/datamap/MVHelper.scala ---
    @@ -118,6 +122,43 @@ object MVHelper {
         DataMapStoreManager.getInstance().saveDataMapSchema(dataMapSchema)
       }
     
    +  private def validateMVQuery(sparkSession: SparkSession,
    +      logicalPlan: LogicalPlan) {
    +    val dataMapProvider = DataMapManager.get().getDataMapProvider(null,
    +      new DataMapSchema("", DataMapClassProvider.MV.getShortName), sparkSession)
    +    dataMapProvider
    +    var catalog = DataMapStoreManager.getInstance().getDataMapCatalog(dataMapProvider,
    +      DataMapClassProvider.MV.getShortName).asInstanceOf[SummaryDatasetCatalog]
    +    if (catalog == null) {
    +      catalog = new SummaryDatasetCatalog(sparkSession)
    +    }
    +    val modularPlan =
    +      catalog.mvSession.sessionState.modularizer.modularize(
    +        catalog.mvSession.sessionState.optimizer.execute(logicalPlan)).next().semiHarmonized
    +
    +    val isValid = modularPlan match {
    +      case g: GroupBy =>
    +        // Make sure all predicates are present in projections.
    +        g.predicateList.forall{p =>
    +          g.outputList.exists{
    +            case a: Alias =>
    +              a.semanticEquals(p) || a.child.semanticEquals(p)
    +            case other => other.semanticEquals(p)
    +          }
    +        }
    +      case _ => true
    +    }
    +    if (!isValid) {
    +      throw new UnsupportedOperationException("Group by columns must be present in project columns")
    +    }
    +    if(catalog.isMVWithSameQueryPresent(logicalPlan)) {
    +      throw new UnsupportedOperationException("MV with same query present")
    +    }
    +    if (!modularPlan.isSPJGH)  {
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2478#discussion_r204479283
 
    --- Diff: datamap/mv/core/src/main/scala/org/apache/carbondata/mv/datamap/MVHelper.scala ---
    @@ -118,6 +122,43 @@ object MVHelper {
         DataMapStoreManager.getInstance().saveDataMapSchema(dataMapSchema)
       }
     
    +  private def validateMVQuery(sparkSession: SparkSession,
    +      logicalPlan: LogicalPlan) {
    +    val dataMapProvider = DataMapManager.get().getDataMapProvider(null,
    +      new DataMapSchema("", DataMapClassProvider.MV.getShortName), sparkSession)
    +    dataMapProvider
    +    var catalog = DataMapStoreManager.getInstance().getDataMapCatalog(dataMapProvider,
    +      DataMapClassProvider.MV.getShortName).asInstanceOf[SummaryDatasetCatalog]
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2478#discussion_r204479337
 
    --- Diff: datamap/mv/core/src/main/scala/org/apache/carbondata/mv/datamap/MVHelper.scala ---
    @@ -118,6 +122,43 @@ object MVHelper {
         DataMapStoreManager.getInstance().saveDataMapSchema(dataMapSchema)
       }
     
    +  private def validateMVQuery(sparkSession: SparkSession,
    +      logicalPlan: LogicalPlan) {
    +    val dataMapProvider = DataMapManager.get().getDataMapProvider(null,
    +      new DataMapSchema("", DataMapClassProvider.MV.getShortName), sparkSession)
    +    dataMapProvider
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2478#discussion_r204479352
 
    --- Diff: datamap/mv/core/src/main/scala/org/apache/carbondata/mv/datamap/MVHelper.scala ---
    @@ -118,6 +122,43 @@ object MVHelper {
         DataMapStoreManager.getInstance().saveDataMapSchema(dataMapSchema)
       }
     
    +  private def validateMVQuery(sparkSession: SparkSession,
    +      logicalPlan: LogicalPlan) {
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2478#discussion_r204479397
 
    --- Diff: datamap/mv/core/src/main/scala/org/apache/carbondata/mv/datamap/MVHelper.scala ---
    @@ -118,6 +122,43 @@ object MVHelper {
         DataMapStoreManager.getInstance().saveDataMapSchema(dataMapSchema)
       }
     
    +  private def validateMVQuery(sparkSession: SparkSession,
    +      logicalPlan: LogicalPlan) {
    +    val dataMapProvider = DataMapManager.get().getDataMapProvider(null,
    +      new DataMapSchema("", DataMapClassProvider.MV.getShortName), sparkSession)
    +    dataMapProvider
    +    var catalog = DataMapStoreManager.getInstance().getDataMapCatalog(dataMapProvider,
    +      DataMapClassProvider.MV.getShortName).asInstanceOf[SummaryDatasetCatalog]
    +    if (catalog == null) {
    +      catalog = new SummaryDatasetCatalog(sparkSession)
    +    }
    +    val modularPlan =
    +      catalog.mvSession.sessionState.modularizer.modularize(
    +        catalog.mvSession.sessionState.optimizer.execute(logicalPlan)).next().semiHarmonized
    +
    +    val isValid = modularPlan match {
    +      case g: GroupBy =>
    +        // Make sure all predicates are present in projections.
    +        g.predicateList.forall{p =>
    +          g.outputList.exists{
    +            case a: Alias =>
    +              a.semanticEquals(p) || a.child.semanticEquals(p)
    +            case other => other.semanticEquals(p)
    +          }
    +        }
    +      case _ => true
    +    }
    +    if (!isValid) {
    +      throw new UnsupportedOperationException("Group by columns must be present in project columns")
    +    }
    +    if(catalog.isMVWithSameQueryPresent(logicalPlan)) {
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2478
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6176/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2478
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6189/



---

[qiuchenjian-2]

Github user jackylk commented on the issue:

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


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---