Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 )
Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries ...................................................................... Patch Set 2: (9 comments) http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1042 PS5, Line 1042: failed_thrift_profiles.emplace_back(*thrift_profi > nit: this is already done in __set_failed_thrift_profiles() Done http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1052 PS5, Line 1052: original_profiles.emplace_back(ss->str()); > nit: this is already done in __set_failed_profiles() Done http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1056 PS5, Line 1056: > nit: only the 'format' of the request is used. We can refactor it to a TRun Done http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1121 PS5, Line 1121: GetRuntimeProfileOu > nit: 4 spaces indention Not sure why, but ClangFormat makes it like this. http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1132 PS5, Line 1132: if (was_retried && request.include_query_attempts) { : if (request.format == TRuntimeProfileFormat::THRIFT) { : SetFailedProfile(&thrift_profile, return_val); : } else if (request.format == TRuntimeProfileFormat::JSON) { : JSONProfileToStringProfile(&ss, &json_profile); : SetFailedProfile(&ss, return_val); : } else { : DCHECK(request.format == TRuntimeProfileFormat::STRING : || request.format == TRuntimeProfileFormat::BASE64); : > nit: the 3 if branches have the same conditions as SetProfile() at line 105 Done http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py@110 PS5, Line 110: ons > typo then? Done http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py@111 PS5, Line 111: thi > typo then? Done http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py@1004 PS5, Line 1004: db_ > nit: 2 spaces indention Done http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py@1071 PS5, Line 1071: prettytable = self.construct_table_with_header(column_names) : formatter = PrettyOutputFormatter(pret > nit: may be helpful to print the three valid values as well. Done -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com> Gerrit-Comment-Date: Wed, 16 Sep 2020 01:49:25 +0000 Gerrit-HasComments: Yes
