> On April 8, 2015, 10:55 p.m., Hanifi Gunes wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinTemplate.java, > > line 39 > > <https://reviews.apache.org/r/32945/diff/1/?file=920309#file920309line39> > > > > is this instance variable needed when we are holding left instance? can > > we localize?
I will make a local copy of this member for performance but I think having this as a class member is ok here. This is used in two cases 1. first time we invoke outputRecords(), we use the record count to determine if we need to invoke populateOutgoingBatch() otherwise we'll still go over all the records in the right. 2. Once we have processed one left batch this is also useful to check if we need to end processing. If I remove this I will have to maintain some other state like 'isFirst' in the template which does not serve the purpose. > On April 8, 2015, 10:55 p.m., Hanifi Gunes wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinTemplate.java, > > line 98 > > <https://reviews.apache.org/r/32945/diff/1/?file=920309#file920309line98> > > > > Afaik any non-long integer arithmetic results in an int by default. We > > may save some cpu cycles if we declare nextRightRecordToProcess be short > > and get rid of bit masking here. > > > > Also is there any reason for not altering method signature to > > emitRight(compositeIndex, recordIndex, outIndex) instead of making this > > computation for each tuple? Good catch. - Mehant ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32945/#review79434 ----------------------------------------------------------- On April 9, 2015, 9:03 p.m., Mehant Baid wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32945/ > ----------------------------------------------------------- > > (Updated April 9, 2015, 9:03 p.m.) > > > Review request for drill and Aman Sinha. > > > Repository: drill-git > > > Description > ------- > > This patch implements the nested loop join operator. The main changes are in > the files NestedLoopJoinBatch and NestedLoopJoinTemplate. This patch only > contains the execution changes. Planning patch will be posted in a separate > review request by Aman. > > > Diffs > ----- > > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractPhysicalVisitor.java > 27b0ecb > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/PhysicalVisitor.java > e6a89d0 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/NestedLoopJoinPOP.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoin.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatchCreator.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinTemplate.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/record/ExpandableHyperContainer.java > 90310e2 > protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java > 9a9d196 > protocol/src/main/protobuf/UserBitShared.proto 5e44655 > > Diff: https://reviews.apache.org/r/32945/diff/ > > > Testing > ------- > > The tests are dependent on the planning changes, hence not uploaded as part > of this patch. However > https://github.com/mehant/drill/blob/notin_1/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestNestedLoopJoin.java > is a working branch that contains a bunch of tests (planning & execution) > added for nested loop join. > > > Thanks, > > Mehant Baid > >
