impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5481: Clarify RowDescriptor ownership
Date Fri, 16 Jun 2017 19:22:55 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5481: Clarify RowDescriptor ownership
......................................................................


Patch Set 2:

(2 comments)

The change makes sense to me, I think we're just missing some documentation about the assumed
lifetime of RowDescriptors. Implicitly the code relies on the assumption that they have the
same lifetime as the plan tree, which appears to be valid, but isn't documented anywhere.

http://gerrit.cloudera.org:8080/#/c/7206/2/be/src/runtime/data-stream-recvr.h
File be/src/runtime/data-stream-recvr.h:

Line 134:   /// Row schema, copied from the caller of CreateRecvr().
No longer accurate


http://gerrit.cloudera.org:8080/#/c/7206/2/be/src/runtime/row-batch.h
File be/src/runtime/row-batch.h:

Line 428:   const RowDescriptor* row_desc_;
Maybe comment on what the expected lifetime of row_desc_ is? Alternatively we could add a
comment to the RowDescriptor class, or do something else like store the RowDescriptors in
an object pool.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2fc39170f775581d406b6a97445046f508d8d75f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message