[GitHub] carbondata pull request #2444: [CARBONDATA-2686] Implement Left join on MV d...

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2444#discussion_r200568031
 
    --- Diff: datamap/mv/plan/src/main/scala/org/apache/carbondata/mv/plans/modular/ModularRelation.scala ---
    @@ -86,10 +87,28 @@ object HarmonizedRelation {
           Select(_, _, _, _, _, dim :: Nil, NoFlags, Nil, Nil, _),
           NoFlags,
           Nil, _) if (dim.isInstanceOf[ModularRelation]) =>
    -        if (g.outputList
    -          .forall(col => col.isInstanceOf[AttributeReference] ||
    -                         (col.isInstanceOf[Alias] &&
    -                          col.asInstanceOf[Alias].child.isInstanceOf[AttributeReference]))) {
    +        if (g.outputList.forall(col => {
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2444#discussion_r200568141
 
    --- Diff: datamap/mv/plan/src/main/scala/org/apache/carbondata/mv/plans/modular/Harmonizer.scala ---
    @@ -36,17 +36,33 @@ abstract class Harmonizer(conf: SQLConf)
       def batches: Seq[Batch] = {
         Batch(
           "Data Harmonizations", fixedPoint,
    -      HarmonizeDimensionTable,
    -      HarmonizeFactTable) :: Nil
    +      Seq( HarmonizeDimensionTable) ++
    +      extendedOperatorHarmonizationRules: _*) :: Nil
    +//      HarmonizeFactTable) :: Nil
       }
    +
    +  /**
    +   * Override to provide additional rules for the modular operator harmonization batch.
    +   */
    +  def extendedOperatorHarmonizationRules: Seq[Rule[ModularPlan]] = Nil
    +}
    +
    +/**
    + * A full Harmonizer - harmonize both fact and dimension tables
    + */
    +object FullHarmonizer extends FullHarmonizer
    +
    +class FullHarmonizer extends Harmonizer(new SQLConf()) {
    +  override def extendedOperatorHarmonizationRules: Seq[Rule[ModularPlan]] =
    +    super.extendedOperatorHarmonizationRules ++ (HarmonizeFactTable :: Nil)
     }
     
     /**
    - * An default Harmonizer
    + * A semi Harmonizer - harmonize dimension tables only
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2444#discussion_r200568159
 
    --- Diff: datamap/mv/plan/src/main/scala/org/apache/carbondata/mv/plans/modular/Harmonizer.scala ---
    @@ -36,17 +36,33 @@ abstract class Harmonizer(conf: SQLConf)
       def batches: Seq[Batch] = {
         Batch(
           "Data Harmonizations", fixedPoint,
    -      HarmonizeDimensionTable,
    -      HarmonizeFactTable) :: Nil
    +      Seq( HarmonizeDimensionTable) ++
    +      extendedOperatorHarmonizationRules: _*) :: Nil
    +//      HarmonizeFactTable) :: Nil
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2444#discussion_r200568172
 
    --- Diff: datamap/mv/core/src/main/scala/org/apache/carbondata/mv/rewrite/DefaultMatchMaker.scala ---
    @@ -245,15 +269,22 @@ object SelectSelectNoChildDelta extends DefaultMatchPattern with PredicateHelper
                       }
                   }
                   val tPredicateList = sel_1q.predicateList.filter { p =>
    -                !sel_1a.predicateList.exists(_.semanticEquals(p)) }
    -                val wip = sel_1q.copy(
    -                  predicateList = tPredicateList,
    -                  children = tChildren,
    -                  joinEdges = tJoinEdges.filter(_ != null),
    -                  aliasMap = tAliasMap.toMap)
    -
    -                val done = factorOutSubsumer(wip, usel_1a, wip.aliasMap)
    -                Seq(done)
    +                !sel_1a.predicateList.exists(_.semanticEquals(p))
    +              } ++ (if (isLeftJoinView(sel_1a) &&
    +                        sel_1q.joinEdges.head.joinType == Inner) {
    +                sel_1a.children(1)
    +                  .asInstanceOf[HarmonizedRelation].tag.map(IsNotNull(_)).toSeq
    +              } else {
    +                Seq.empty
    +              })
    +              val wip = sel_1q.copy(
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2444#discussion_r200568187
 
    --- Diff: datamap/mv/core/src/main/scala/org/apache/carbondata/mv/rewrite/DefaultMatchMaker.scala ---
    @@ -127,8 +126,27 @@ object SelectSelectNoChildDelta extends DefaultMatchPattern with PredicateHelper
         }
       }
     
    -  def apply(
    -      subsumer: ModularPlan,
    +  private def isLeftJoinView(subsumer: ModularPlan): Boolean = {
    +    if (subsumer.isInstanceOf[modular.Select]) {
    +      val sel = subsumer.asInstanceOf[modular.Select]
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user jackylk commented on the issue:

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


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---