Tim Armstrong has posted comments on this change. Change subject: IMPALA-2550: Switch to per-query exec rpc ......................................................................
Patch Set 6: (4 comments) Looked through some of the files I'm more familiar with - had a few minor comments. http://gerrit.cloudera.org:8080/#/c/6535/4/be/src/common/status.h File be/src/common/status.h: Line 22: #include <ostream> Can you include iosfwd instead. status.h is included everywhere and it would be nice not to pull in and compile the huge iostream headers for every file in the codebase. http://gerrit.cloudera.org:8080/#/c/6535/6/be/src/runtime/runtime-state.h File be/src/runtime/runtime-state.h: Line 340: /// only populated by test c'tor Isn't it also used by the fe-support code path? http://gerrit.cloudera.org:8080/#/c/6535/6/be/src/runtime/test-env.cc File be/src/runtime/test-env.cc: Line 23: #include "common/names.h" common/names.h usually gets put below the other includes because it checks to see what other headers were pulled in. Line 61: if (buffer_pool_min_buffer_len_ != -1 && buffer_pool_capacity_ != -1) { Thanks for fixing this. -- To view, visit http://gerrit.cloudera.org:8080/6535 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I20769e420711737b6b385c744cef4851cee3facd Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
