[GitHub] carbondata pull request #2302: [CARBONDATA-2475] Support Modular Core for Ma...

[qiuchenjian-2]

GitHub user ravipesala opened a pull request:

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

    [CARBONDATA-2475] Support Modular Core for Materialized View DataMap for query matching and rewriting

    Currently carbon supports preaggregate datamap, which only supports preaggregate on single table. To improve it, we can add join capability by implementing Materialized View.
   
    In carbon Materialized View, Modular Core provides the plan matcher and rewrites the query with MV query.
   
    Be sure to do all of the following checklist to help us incorporate
    your contribution quickly and easily:
   
     - [ ] Any interfaces changed?
     
     - [ ] Any backward compatibility impacted?
     
     - [ ] Document update required?
   
     - [ ] Testing done
            Please provide details on
            - Whether new unit test cases have been added or why no new tests are required?
            - How it is tested? Please attach test report.
            - Is it a performance related change? Please attach the performance test report.
            - Any additional information to help reviewers in testing this change.
           
     - [ ] 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/ravipesala/incubator-carbondata mv-core

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

    https://github.com/apache/carbondata/pull/2302.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 #2302
   
----
commit fb74f53b4b585a4b92cfcd57c4d7a8da745c870f
Author: ravipesala <ravi.pesala@...>
Date:   2018-05-12T17:19:19Z

     Support Modular Core for Materialized View DataMap

commit de8ee65f2fc21159aabb636afbe68495adf96df6
Author: ravipesala <ravi.pesala@...>
Date:   2018-05-12T05:11:01Z

    Integrate MV DataMap to Carbon

----


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2302#discussion_r187779386
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datamap/DataMapStoreManager.java ---
    @@ -208,7 +208,7 @@ public DataMapCatalog getDataMapCatalog(DataMapProvider dataMapProvider, String
       }
     
       /**
    -   * Initialize by reading all datamaps from store and re register it
    +   * Intialize by reading all datamaps from store and re register it
    --- End diff --
   
    typo


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2302#discussion_r187779404
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datamap/status/DataMapStatusManager.java ---
    @@ -53,6 +53,22 @@ private DataMapStatusManager() {
         return storageProvider.getDataMapStatusDetails();
       }
     
    +  /**
    +   * Get enabled datamap status details
    +   * @return
    +   * @throws IOException
    +   */
    +  public static DataMapStatusDetail[] getEnabledDataMapStatusDetails() throws IOException {
    --- End diff --
   
    I think this is unused function, no need to add


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2302#discussion_r187779425
 
    --- Diff: datamap/mv/core/src/test/scala/org/apache/carbondata/mv/rewrite/MVCreateTestCase.scala ---
    @@ -0,0 +1,676 @@
    +package org.apache.carbondata.mv.rewrite
    --- End diff --
   
    please add license.
    license is missing in all testcase files


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2302#discussion_r187779448
 
    --- Diff: datamap/mv/core/src/test/scala/org/apache/carbondata/mv/rewrite/Tpcds_1_4_Suite.scala ---
    @@ -0,0 +1,80 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.mv.rewrite
    +
    +import org.apache.spark.sql.SparkSession
    +import org.apache.spark.sql.test.util.PlanTest
    +import org.scalatest.BeforeAndAfter
    +
    +import org.apache.carbondata.mv.testutil.Tpcds_1_4_Tables._
    +
    +class Tpcds_1_4_Suite extends PlanTest with BeforeAndAfter {
    --- End diff --
   
    Please remove this file as it is all commented out


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2302#discussion_r187779458
 
    --- Diff: datamap/mv/plan/src/test/scala/org/apache/carbondata/mv/plans/Tpcds_1_4_BenchmarkSuite.scala ---
    @@ -0,0 +1,88 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.mv.plans
    +
    +import scala.util.{Failure, Success, Try}
    +
    +import org.apache.spark.sql.SparkSession
    +import org.scalatest.BeforeAndAfter
    +
    +import org.apache.carbondata.mv.dsl._
    +import org.apache.carbondata.mv.testutil.ModularPlanTest
    +
    +// scalastyle:off println
    +class Tpcds_1_4_BenchmarkSuite extends ModularPlanTest with BeforeAndAfter {
    --- End diff --
   
    Please remove this file


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2302#discussion_r187779478
 
    --- Diff: integration/spark2/src/main/spark2.2/org/apache/spark/sql/hive/CarbonAnalyzer.scala ---
    @@ -20,15 +20,33 @@ import org.apache.spark.sql.SparkSession
     import org.apache.spark.sql.catalyst.analysis.Analyzer
     import org.apache.spark.sql.catalyst.catalog.SessionCatalog
     import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
    +import org.apache.spark.sql.catalyst.rules.Rule
     import org.apache.spark.sql.internal.SQLConf
    +import org.apache.spark.util.CarbonReflectionUtils
     
     class CarbonAnalyzer(catalog: SessionCatalog,
         conf: SQLConf,
         sparkSession: SparkSession,
         analyzer: Analyzer) extends Analyzer(catalog, conf) {
    +
    +  val mvPlan = try {
    +    CarbonReflectionUtils.createObject(
    +      "org.apache.carbondata.mv.datamap.MVAnalyzerRule",
    +      sparkSession)._1.asInstanceOf[Rule[LogicalPlan]]
    +  } catch {
    +    case e: Exception =>
    +      null
    +  }
    +
       override def execute(plan: LogicalPlan): LogicalPlan = {
         var logicalPlan = analyzer.execute(plan)
         logicalPlan = CarbonPreAggregateDataLoadingRules(sparkSession).apply(logicalPlan)
    -    CarbonPreAggregateQueryRules(sparkSession).apply(logicalPlan)
    +    logicalPlan = CarbonPreAggregateQueryRules(sparkSession).apply(logicalPlan)
    +    // TODO Get the analyzer rules from registered datamap class and apply here.
    --- End diff --
   
    This TODO can be removed?


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2302#discussion_r187789629
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datamap/status/DataMapStatusManager.java ---
    @@ -53,6 +53,22 @@ private DataMapStatusManager() {
         return storageProvider.getDataMapStatusDetails();
       }
     
    +  /**
    +   * Get enabled datamap status details
    +   * @return
    +   * @throws IOException
    +   */
    +  public static DataMapStatusDetail[] getEnabledDataMapStatusDetails() throws IOException {
    --- End diff --
   
    It is used in `SummaryDatasetCatalog`


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2302#discussion_r187789632
 
    --- Diff: datamap/mv/core/src/test/scala/org/apache/carbondata/mv/rewrite/MVCreateTestCase.scala ---
    @@ -0,0 +1,676 @@
    +package org.apache.carbondata.mv.rewrite
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2302#discussion_r187789635
 
    --- Diff: datamap/mv/core/src/test/scala/org/apache/carbondata/mv/rewrite/Tpcds_1_4_Suite.scala ---
    @@ -0,0 +1,80 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.mv.rewrite
    +
    +import org.apache.spark.sql.SparkSession
    +import org.apache.spark.sql.test.util.PlanTest
    +import org.scalatest.BeforeAndAfter
    +
    +import org.apache.carbondata.mv.testutil.Tpcds_1_4_Tables._
    +
    +class Tpcds_1_4_Suite extends PlanTest with BeforeAndAfter {
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2302#discussion_r187789637
 
    --- Diff: datamap/mv/plan/src/test/scala/org/apache/carbondata/mv/plans/Tpcds_1_4_BenchmarkSuite.scala ---
    @@ -0,0 +1,88 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.mv.plans
    +
    +import scala.util.{Failure, Success, Try}
    +
    +import org.apache.spark.sql.SparkSession
    +import org.scalatest.BeforeAndAfter
    +
    +import org.apache.carbondata.mv.dsl._
    +import org.apache.carbondata.mv.testutil.ModularPlanTest
    +
    +// scalastyle:off println
    +class Tpcds_1_4_BenchmarkSuite extends ModularPlanTest with BeforeAndAfter {
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2302#discussion_r187789640
 
    --- Diff: integration/spark2/src/main/spark2.2/org/apache/spark/sql/hive/CarbonAnalyzer.scala ---
    @@ -20,15 +20,33 @@ import org.apache.spark.sql.SparkSession
     import org.apache.spark.sql.catalyst.analysis.Analyzer
     import org.apache.spark.sql.catalyst.catalog.SessionCatalog
     import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
    +import org.apache.spark.sql.catalyst.rules.Rule
     import org.apache.spark.sql.internal.SQLConf
    +import org.apache.spark.util.CarbonReflectionUtils
     
     class CarbonAnalyzer(catalog: SessionCatalog,
         conf: SQLConf,
         sparkSession: SparkSession,
         analyzer: Analyzer) extends Analyzer(catalog, conf) {
    +
    +  val mvPlan = try {
    +    CarbonReflectionUtils.createObject(
    +      "org.apache.carbondata.mv.datamap.MVAnalyzerRule",
    +      sparkSession)._1.asInstanceOf[Rule[LogicalPlan]]
    +  } catch {
    +    case e: Exception =>
    +      null
    +  }
    +
       override def execute(plan: LogicalPlan): LogicalPlan = {
         var logicalPlan = analyzer.execute(plan)
         logicalPlan = CarbonPreAggregateDataLoadingRules(sparkSession).apply(logicalPlan)
    -    CarbonPreAggregateQueryRules(sparkSession).apply(logicalPlan)
    +    logicalPlan = CarbonPreAggregateQueryRules(sparkSession).apply(logicalPlan)
    +    // TODO Get the analyzer rules from registered datamap class and apply here.
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2302#discussion_r187789644
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datamap/DataMapStoreManager.java ---
    @@ -208,7 +208,7 @@ public DataMapCatalog getDataMapCatalog(DataMapProvider dataMapProvider, String
       }
     
       /**
    -   * Initialize by reading all datamaps from store and re register it
    +   * Intialize by reading all datamaps from store and re register it
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---