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 <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
