impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-3105: avoid overrunning allocated tuple buffer
Date Fri, 08 Apr 2016 19:30:53 GMT
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 <tarmstrong@cloudera.com>
Gerrit-Reviewer: Casey Ching <casey@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: No

Mime
View raw message