[GitHub] carbondata pull request #2335: [CARBONDATA-2573] integrate carbonstore mv br...

2018-06-04 Thread ravipesala
Github user ravipesala commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2335#discussion_r192945546
  
--- Diff: 
datamap/mv/core/src/main/scala/org/apache/carbondata/mv/datamap/MVState.scala 
---
@@ -31,25 +31,25 @@ private[mv] class MVState(summaryDatasetCatalog: 
SummaryDatasetCatalog) {
   // Note: These are all lazy vals because they depend on each other (e.g. 
conf) and we
   // want subclasses to override some of the fields. Otherwise, we would 
get a lot of NPEs.
 
-  /**
-   * Modular query plan modularizer
-   */
-  lazy val modularizer = SimpleModularizer
-
-  /**
-   * Logical query plan optimizer.
-   */
-  lazy val optimizer = BirdcageOptimizer
-
-  lazy val matcher = DefaultMatchMaker
-
-  lazy val navigator: Navigator = new Navigator(summaryDatasetCatalog, 
this)
+//  /**
+//   * Modular query plan modularizer
+//   */
+//  lazy val modularizer = SimpleModularizer
+//
+//  /**
+//   * Logical query plan optimizer.
+//   */
--- End diff --

Not required, removed


---


[GitHub] carbondata pull request #2335: [CARBONDATA-2573] integrate carbonstore mv br...

2018-06-04 Thread ravipesala
Github user ravipesala commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2335#discussion_r192945440
  
--- Diff: 
datamap/mv/core/src/main/scala/org/apache/carbondata/mv/datamap/MVHelper.scala 
---
@@ -373,5 +373,25 @@ object MVHelper {
   case other => other
 }
   }
+
+  def rewriteWithMVTable(rewrittenPlan: ModularPlan, rewrite: 
QueryRewrite): ModularPlan = {
--- End diff --

ok


---


[GitHub] carbondata pull request #2335: [CARBONDATA-2573] integrate carbonstore mv br...

2018-06-04 Thread jackylk
Github user jackylk commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2335#discussion_r192744678
  
--- Diff: 
datamap/mv/core/src/main/scala/org/apache/carbondata/mv/datamap/MVState.scala 
---
@@ -31,25 +31,25 @@ private[mv] class MVState(summaryDatasetCatalog: 
SummaryDatasetCatalog) {
   // Note: These are all lazy vals because they depend on each other (e.g. 
conf) and we
   // want subclasses to override some of the fields. Otherwise, we would 
get a lot of NPEs.
 
-  /**
-   * Modular query plan modularizer
-   */
-  lazy val modularizer = SimpleModularizer
-
-  /**
-   * Logical query plan optimizer.
-   */
-  lazy val optimizer = BirdcageOptimizer
-
-  lazy val matcher = DefaultMatchMaker
-
-  lazy val navigator: Navigator = new Navigator(summaryDatasetCatalog, 
this)
+//  /**
+//   * Modular query plan modularizer
+//   */
+//  lazy val modularizer = SimpleModularizer
+//
+//  /**
+//   * Logical query plan optimizer.
+//   */
--- End diff --

Is it for debugging purpose?


---


[GitHub] carbondata pull request #2335: [CARBONDATA-2573] integrate carbonstore mv br...

2018-06-04 Thread jackylk
Github user jackylk commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2335#discussion_r192743275
  
--- Diff: 
datamap/mv/core/src/main/scala/org/apache/carbondata/mv/datamap/MVHelper.scala 
---
@@ -373,5 +373,25 @@ object MVHelper {
   case other => other
 }
   }
+
+  def rewriteWithMVTable(rewrittenPlan: ModularPlan, rewrite: 
QueryRewrite): ModularPlan = {
--- End diff --

please add comment for this func


---