Alex Behm has posted comments on this change. Change subject: IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder ......................................................................
Patch Set 12: Code-Review+1 (6 comments) I think the patch is ready to have more eyes on it. http://gerrit.cloudera.org:8080/#/c/3873/12/be/src/exec/partitioned-hash-join-builder.h File be/src/exec/partitioned-hash-join-builder.h: Line 45: /// The builder owns the hash tables and build row partitions. The builder partitions remove last "partitions" Line 46: /// first partitions the build rows and builds hash tables for in-memory partitions, I think it's worth going into more detail here: My understanding is that the builder completes the level0 partitioning and construction. After FlushFinal() the builder has produced some in-memory partitions and some spilled partitions. The in-memory partitions have hash tables and the spilled partitions have a probe-side stream prepared with one reserved buffer. Line 47: /// driven by the DataSink interface methods. After FlushFinal() is called, the join the join node drives level1-N re-partitioning, calling into the builder for constructing hash tables and spilling partitions. Line 84: /// partitions in FlushFinal(). are they indexed by partition index? Line 271: /// When this function returns successfully, each partition is in one of these states: Nice! Very clear Line 303: void Codegen(RuntimeState* state); Can codegen not fail? Should return a Status? -- To view, visit http://gerrit.cloudera.org:8080/3873 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1e02ea9c7a7d1a0f373b11aa06c3237e1c7bd4cb Gerrit-PatchSet: 12 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
