Dan Hecht has posted comments on this change. Change subject: IMPALA-3105: avoid overrunning allocated tuple buffer ......................................................................
Patch Set 3: > Reworked the change so that it's less invasive and changes > capacity_ only when the tuple buffer is actually allocated (rather > than when the rowbatch is created). Why is it better to defer it until allocation time? I guess because of the assumptions you allude to about row batches having batch_size() capacity. But is side-stepping that safe/robust? where are these assumptions made? As mentioned earlier, doesn't HdfsScanner::StartNewRowBatch() (and KuduScanner::GetNext() have a similar issue and would benefit from using this)? Also, can DataStreamSender::Channel::Init() use it? 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. -- 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 <[email protected]> Gerrit-Reviewer: Casey Ching <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: No
