Thomas Tauber-Marshall has posted comments on this change.

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


Patch Set 8:

(1 comment)

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

PS8, Line 445:  switch (partitioner_->type()) {
             :       case TPartitionType::HASH_PARTITIONED:
             :         RETURN_IF_ERROR(
             :             
reinterpret_cast<HashStreamPartitioner*>(partitioner_.get())
             :                 ->Partition(batch,
             :                     [this, num_channels](TupleRow* row, uint32_t 
partition) -> Status {
             :                       return this->channels_[partition % 
num_channels]->AddRow(row);
             :                     }));
             :         break;
             :       case TPartitionType::KUDU:
             :         RETURN_IF_ERROR(
             :             
reinterpret_cast<KuduStreamPartitioner*>(partitioner_.get())
             :                 ->Partition(batch,
             :                     [this, num_channels](TupleRow* row, uint32_t 
partition) -> Status {
             :                       return this->channels_[partition % 
num_channels]->AddRow(row);
             :                     }));
             :         break;
> Looking at how this came out in the code, my feeling is that it might be cl
Since we're doing the switching here and calling non-virtual functions anyways, 
the cleanest thing would be to go back to the way it originally worked, with 
one function call per row to get the partition value, just now it would be a 
non-virtual function.


-- 
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: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-HasComments: Yes

Reply via email to