Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-3742: partitions DMLs for Kudu tables
......................................................................


Patch Set 5:

(14 comments)

Still working on performance.

http://gerrit.cloudera.org:8080/#/c/6037/4/be/src/runtime/data-stream-partitioner.cc
File be/src/runtime/data-stream-partitioner.cc:

PS4, Line 62: e_de
> nullptr
Done


Line 66: 
> how about using kudu::client here, so you don't have to write it out a furt
Done


Line 80:     return Status::OK();
> override
Done


PS4, Line 88: i
> better var name than 'p'
Done


PS4, Line 102: 
> needs a comment, like all the other members.
Done


PS4, Line 104: // Ku
> remove std::
Done


PS4, Line 108: or<int> refe
> next_channel_
Done


PS4, Line 119: s HashStreamPartitioner : public DataStreamPartitioner 
> for (ExprContext* ctx : partition_expr_ctxs_) {
Done


PS4, Line 135: 
> remove std::
Done


PS4, Line 144: se TPartitionType::HASH_PART
> more idiomatic now to use Substitute() on line 152.
Done


PS4, Line 146: break;
> auto
Done


http://gerrit.cloudera.org:8080/#/c/6037/4/be/src/runtime/data-stream-sender.cc
File be/src/runtime/data-stream-sender.cc:

PS4, Line 390: TURN
> probably easier to write if (partitioner_)
Done


Line 418: 
> one line
Done


Line 480: Status DataStreamSender::SerializeBatch(RowBatch* src, TRowBatch* 
dest, int num_receivers) {
> one line
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic10b3295159354888efcde3df76b0edb24161515
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-HasComments: Yes

Reply via email to