impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marcel Kornacker (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3567: Part 1: groundwork to make Join build sides DataSinks
Date Mon, 08 Aug 2016 23:27:27 GMT
Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-3567: Part 1: groundwork to make Join build sides DataSinks
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3842/2/be/src/exec/nested-loop-join-builder.cc
File be/src/exec/nested-loop-join-builder.cc:

Line 31:     MemTracker* mem_tracker) :
i think the convention is to have the : on the new line as well


http://gerrit.cloudera.org:8080/#/c/3842/1/be/src/exec/nested-loop-join-builder.h
File be/src/exec/nested-loop-join-builder.h:

Line 35: /// tuple memory, the non-copying mode is used and row batches are simply accumulated
in
> It switches depending on whether the row batches have the 'need_to_return' 
but isn't it true that the sink is only driven in one of two ways, Send or AddBuildBatch?
ie, if one caller uses Send that same caller will never call AddBuildBatch directly?


http://gerrit.cloudera.org:8080/#/c/3842/2/be/src/exec/nested-loop-join-builder.h
File be/src/exec/nested-loop-join-builder.h:

Line 91:   RowBatchList raw_build_batches_;
i still think raw is a misnomer and 'original' or 'input' is clearer.


-- 
To view, visit http://gerrit.cloudera.org:8080/3842
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9d7608181eeacfe706a09c1e153d0a3e1ee9b475
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message