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