Michal Ostrowski has posted comments on this change. ( http://gerrit.cloudera.org:8080/11425 )
Change subject: IMPALA-2063 Remove newline characters in query status. ...................................................................... Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/11425/5/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/11425/5/be/src/util/runtime-profile.cc@545 PS5, Line 545: StripTrailingWhitespace(&value); : StripDupCharacters(&value, '\n', 0); : : lock_guard<SpinLock> l(info_strings_lock_); : I > Done Done http://gerrit.cloudera.org:8080/#/c/11425/5/be/src/util/runtime-profile.cc@551 PS5, Line 551: info_strings_.emplace(key, std::move(value)); > I applied the patch and looked at the query profile output at the debug web That's fine. The only reason the double newline is being stripped is because it's the only way to make a sane test. If I take this out, I have to remove the test code as well -- and I really don't see the value of adding in a specific test to validate that "Status: CANCELLED" doesn't have extra newlines (especially since we don't test for any other formatting issues). http://gerrit.cloudera.org:8080/#/c/11425/5/be/src/util/runtime-profile.cc@551 PS5, Line 551: info_strings_.emplace(key, std::move(value)); : info_strings_display_order_.push_back(key); : } else { : if (append) { : > Done Done -- To view, visit http://gerrit.cloudera.org:8080/11425 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2bbd7d7fe2c6d0f3799d0e6b336710bccfef0ab1 Gerrit-Change-Number: 11425 Gerrit-PatchSet: 8 Gerrit-Owner: Michal Ostrowski <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Michal Ostrowski <[email protected]> Gerrit-Comment-Date: Tue, 02 Oct 2018 12:25:41 +0000 Gerrit-HasComments: Yes
