impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <>
Subject [hs2client-CR] Implemented a framework for the C++ hiveserver2 client.
Date Wed, 30 Mar 2016 05:44:26 GMT
Matthew Jacobs has posted comments on this change.

Change subject: Implemented a framework for the C++ hiveserver2 client.

Patch Set 3:


Thanks! I still need to look a bit more in the morning.
Commit Message:

Line 7: Implemented a framework
Maybe "Initial structure for the hiveserver2 client"?
File src/hs2client/columnar-row-set.h:

Line 26: const void* nulls
how does this work?

Line 28: protected
if we need non-public access then this should be a class rather than a struct

Line 37: ColumnImpl
Can you add the definition of this? It's hard to reason about the columns without the .cc
as well.
File src/hs2client/hs2service.h:

Line 36: GetOption
what happens if key isn't in the map? maybe better to return bool and the value as an out

Line 38: map
const map<>&

Line 83: .
can you add something like: "Called by Connect() before returned to the user."?
File src/hs2client/operation.h:

Line 80: 10
why 10?

Line 80: int
If this is a class constant typically its static const & the declaration goes in the header
but the definition goes in the .cc, e.g.:

static const int DEFAULT_MAX_ROWS; // in the .h
Operation::DEFAULT_MAX_ROWS = 1024; // in the .cc

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I7be82d5897ad6093116915b17924674c4ae508a0
Gerrit-PatchSet: 3
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