impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <>
Subject [hs2client-CR] Initial structure for the C++ hiveserver2 client.
Date Fri, 22 Apr 2016 22:49:52 GMT
Matthew Jacobs has posted comments on this change.

Change subject: Initial structure for the C++ hiveserver2 client.

Patch Set 8:


Not a full pass over this changeset, but I wanted to get the columnar row changes to you since
that's non-trivial.
File src/hs2client/

Line 110: assert(impl);
any reason string is the only one to have this assert? also were you going to switch to using
File src/hs2client/columnar-row-set.h:

Line 27:   
nit: no tab, here and below. I think you can remove this if you look at my gist linked below.

Line 61: class
Per our discussion, we can make the Column classes more concise by using templates. We should
also encapsulate the fields.

I started playing with this here:

Line 61: Column
Please add a good comment here explaining how this is used, including the data layout, and
give a very brief example.

Line 63: ~
FYI destructors need to be virtual if there will ever be subclasses. In this case I don't
think you need any destructor defined.

Line 72:  protected:
       :   // Hides Thrift objects from the header.
       :   struct ColumnImpl;
       :   // For access to the c'tor.
       :   friend class ColumnarRowSet;
No longer necessary
File src/hs2client/

Line 23: logging
this file doesn't exist in the review

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I7be82d5897ad6093116915b17924674c4ae508a0
Gerrit-PatchSet: 8
Gerrit-Project: hs2client
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-Reviewer: Thomas Tauber-Marshall <>
Gerrit-Reviewer: Wes McKinney <>
Gerrit-HasComments: Yes

View raw message