[Impala-ASF-CR] IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder

["Alex Behm (Code Review)" <ge...@cloudera.org>]

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 <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes