[Impala-ASF-CR] IMPALA-3873: Add QueryStateAccessor

["Tim Armstrong (Code Review)" <ge...@cloudera.org>]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-3873: Add QueryStateAccessor
......................................................................


Patch Set 2: Code-Review+1

(6 comments)

Renewed +1, did another pass and noticed a few minor things.

http://gerrit.cloudera.org:8080/#/c/3851/2/be/src/service/impala-http-handler.cc
File be/src/service/impala-http-handler.cc:

Line 757
Was the query_id key deliberately omitted? I'm guessing this was unused?


Line 43: using std::unique_ptr;
Should we add this to common/names.h?


PS2, Line 180: succesfully
successfully


PS2, Line 691: .get()
I don't feel strongly about this, but the .get() isn't necessary for NULL comparisons.


http://gerrit.cloudera.org:8080/#/c/3851/2/be/src/service/query-state-accessor.cc
File be/src/service/query-state-accessor.cc:

Line 161:     return unique_ptr<QueryStateAccessor>(new QueryExecStateAccessor(exec_state));
Nit: I find make_unique<QueryStateAccessor>(exec_state) more readable.


Line 168:     return unique_ptr<QueryStateAccessor>(
Could use make_unique to get this on one line.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3dae66a81988c99cde1516ff511186e17dd8c0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes