> On July 28, 2015, 10:44 p.m., pengcheng xiong wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinToMultiJoinRule.java, > > line 250 > > <https://reviews.apache.org/r/36897/diff/1/?file=1023973#file1023973line250> > > > > Please correct me if my understanding is wrong: it seems that > > systemFieldList and childSystemFieldList may be removed from the input of > > function isCombinablePredicate because they are neither meaningfully used > > nor changed inside the function. A better way is to pass join, left, > > condition and otherCondition and then just use > > constructJoinPredicateInfo(join, condition); and > > constructJoinPredicateInfo(left, otherCondition);
I fixed it, you were right. I needed to refactor the methods as we may have a child that is a HiveJoin or a MultiHiveJoin. It is done in the new version of the patch. > On July 28, 2015, 10:44 p.m., pengcheng xiong wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinToMultiJoinRule.java, > > line 266 > > <https://reviews.apache.org/r/36897/diff/1/?file=1023973#file1023973line266> > > > > Previous version is using equals to compare, and now it is changed to > > containsAll. Can you add some comments explaining why > > childKey.containsAll(keys) is the correct and sufficient condition for > > combining predicates? While I was working on the comments for the code, I realized it was still not right. Now I left comments and should be completely right. Please, let me know what you think. - Jesús ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36897/#review93363 ----------------------------------------------------------- On July 29, 2015, 11:58 a.m., Jesús Camacho Rodríguez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36897/ > ----------------------------------------------------------- > > (Updated July 29, 2015, 11:58 a.m.) > > > Review request for hive and pengcheng xiong. > > > Bugs: HIVE-11257 > https://issues.apache.org/jira/browse/HIVE-11257 > > > Repository: hive-git > > > Description > ------- > > CBO: Calcite Operator To Hive Operator (Calcite Return Path): Method > isCombinablePredicate in HiveJoinToMultiJoinRule should be extended to > support MultiJoin operators merge > > > Diffs > ----- > > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinToMultiJoinRule.java > d0a29a76652c8af120a6efb252c75282730ef097 > > Diff: https://reviews.apache.org/r/36897/diff/ > > > Testing > ------- > > > Thanks, > > Jesús Camacho Rodríguez > >
