Tim Armstrong has posted comments on this change.

Change subject: IMPALA-3105: avoid overrunning allocated tuple buffer
......................................................................


Patch Set 3:

> As mentioned earlier, doesn't HdfsScanner::StartNewRowBatch() (and 
> KuduScanner::GetNext() have a similar issue and would benefit from using 
> this)?  
Made that change in the above places. I did a grep through the codebase for 
GetRowSize() to find other places with similar logic.


> Also, can DataStreamSender::Channel::Init() use it?
I don't think in a straightforward way, since DSS has its own target size for 
rowbatches that is < 8mb and it will create batches larger than the default 
batch size.

> Should we move the NULL check inside the new routine and have it return the 
> status? To get rid of boiler plate code and make it impossible for the caller 
> to do the null check.
Done

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idfd9cd681875821c1c379d97586d3f4850aae622
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Casey Ching <ca...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-HasComments: No

Reply via email to