> On Oct. 29, 2014, 5:09 p.m., Xuefu Zhang wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/SkewJoinOptimizer.java, > > line 626 > > <https://reviews.apache.org/r/27283/diff/2/?file=739757#file739757line626> > > > > static method? Maybe we can create such clone() method in QBJoinTree > > class.
Yes I was thinking add such a method to QBJoinTree. Do you think we should make QBJoinTree clonable and implement deep copy? > On Oct. 29, 2014, 5:09 p.m., Xuefu Zhang wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/SkewJoinOptimizer.java, > > line 694 > > <https://reviews.apache.org/r/27283/diff/2/?file=739757#file739757line694> > > > > The method seems to be recursive, but I'm not sure it's correct. We > > call the same method for the parents and children of the current operator. > > When a parent or child operator of the current operator is processed, > > wouldn't the process go back to the current operator again? Recursion > > should be done in one direction and should have clear end condition. Thanks a lot for the catch. The method can work the way it's being used. But you're right, I will make it more robust. - Rui ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27283/#review59001 ----------------------------------------------------------- On Oct. 29, 2014, 2:59 a.m., Rui Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27283/ > ----------------------------------------------------------- > > (Updated Oct. 29, 2014, 2:59 a.m.) > > > Review request for hive, Szehon Ho and Xuefu Zhang. > > > Bugs: HIVE-8610 > https://issues.apache.org/jira/browse/HIVE-8610 > > > Repository: hive-git > > > Description > ------- > > This patch adds QBJoinTree and colExprMap for the cloned join operator tree > in SkewJoinOptimizer, so that CommonJoinResolver can properly convert the > cloned join to map join. > The added tests are copied from skewjoinopt*.q, except that auto map join is > enabled. > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/SkewJoinOptimizer.java > e87c41b > ql/src/test/queries/clientpositive/skewjoin_mapjoin1.q PRE-CREATION > ql/src/test/queries/clientpositive/skewjoin_mapjoin10.q PRE-CREATION > ql/src/test/queries/clientpositive/skewjoin_mapjoin11.q PRE-CREATION > ql/src/test/queries/clientpositive/skewjoin_mapjoin2.q PRE-CREATION > ql/src/test/queries/clientpositive/skewjoin_mapjoin3.q PRE-CREATION > ql/src/test/queries/clientpositive/skewjoin_mapjoin4.q PRE-CREATION > ql/src/test/queries/clientpositive/skewjoin_mapjoin5.q PRE-CREATION > ql/src/test/queries/clientpositive/skewjoin_mapjoin6.q PRE-CREATION > ql/src/test/queries/clientpositive/skewjoin_mapjoin7.q PRE-CREATION > ql/src/test/queries/clientpositive/skewjoin_mapjoin8.q PRE-CREATION > ql/src/test/queries/clientpositive/skewjoin_mapjoin9.q PRE-CREATION > ql/src/test/results/clientpositive/skewjoin_mapjoin1.q.out PRE-CREATION > ql/src/test/results/clientpositive/skewjoin_mapjoin10.q.out PRE-CREATION > ql/src/test/results/clientpositive/skewjoin_mapjoin11.q.out PRE-CREATION > ql/src/test/results/clientpositive/skewjoin_mapjoin2.q.out PRE-CREATION > ql/src/test/results/clientpositive/skewjoin_mapjoin3.q.out PRE-CREATION > ql/src/test/results/clientpositive/skewjoin_mapjoin4.q.out PRE-CREATION > ql/src/test/results/clientpositive/skewjoin_mapjoin5.q.out PRE-CREATION > ql/src/test/results/clientpositive/skewjoin_mapjoin6.q.out PRE-CREATION > ql/src/test/results/clientpositive/skewjoin_mapjoin7.q.out PRE-CREATION > ql/src/test/results/clientpositive/skewjoin_mapjoin8.q.out PRE-CREATION > ql/src/test/results/clientpositive/skewjoin_mapjoin9.q.out PRE-CREATION > > Diff: https://reviews.apache.org/r/27283/diff/ > > > Testing > ------- > > > Thanks, > > Rui Li > >
