----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14958/#review28628 -----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java <https://reviews.apache.org/r/14958/#comment55542> Why is this here? It isn't used by the MergingRecordBatch. Additionally, doCompare and doCopy should preferrably only managed internally to the compiled version of the RecordMergeTemplate. exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java <https://reviews.apache.org/r/14958/#comment55543> These should be static. That was a mistake we made previously. They hold state. exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java <https://reviews.apache.org/r/14958/#comment55541> It seems like you are doing a large amount of logic here rather than doing it in the template. I believe this will substantially reduce the likelihood of everything getting inlined. All the tight loops should be in the template. exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java <https://reviews.apache.org/r/14958/#comment55540> Unless I'm misunderstanding, you should probably use existing func calls for this purpose rather than reimplementing the comparison logic. Let's discuss. - Jacques Nadeau On Oct. 30, 2013, 12:06 a.m., Ben Becker wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14958/ > ----------------------------------------------------------- > > (Updated Oct. 30, 2013, 12:06 a.m.) > > > Review request for drill. > > > Bugs: DRILL-229 > https://issues.apache.org/jira/browse/DRILL-229 > > > Repository: drill-git > > > Description > ------- > > Implements the merging receiver operator as described in DRILL-229. > > Commit Log: > > - Fix multi-batch bugs. Add human-readable test output. > - Fix recordCount and valueIndex off-by-one > - Implement manual comparison code generation. Manual approach was chosen > over modifying the ValueVectorReadExpression, EvalutionVisitor and > CodeGenerator to minimize special-case code in those classes. > - Implemented code gen for copying; comparison needs a materializer that > works on a RecordBatchLoader > - Flesh out the reset of the merging receiver and fix bugs. Next step is > generated code for comparator and copier. > - Implements support for RecordBatchLoader, VectorContainer and BatchSchema > in MergingRecordBatch Implements first-iteration specific logic WIP: Merge > logic. Some constructs are in place, others are temporary or only stubbed > out) > - Add SingleMergeExchange, which constructs one receiver and many senders > (similar to UnionExchange) > - Implement stub for MergingRecordBatch > - Add test for MergingReceiver (leveraging SingleMergeExchange) > s/MergingReceiver/MergingReceiverPOP/ > - Implement basic merging POP config and creator > > > Diffs > ----- > > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/MergingReceiverPOP.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/SingleMergeExchange.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java > PRE-CREATION > exec/java-exec/src/test/resources/mergerecv/merging_receiver.json > PRE-CREATION > > Diff: https://reviews.apache.org/r/14958/diff/ > > > Testing > ------- > > > Thanks, > > Ben Becker > >
