Gokul Kolady has posted comments on this change. ( http://gerrit.cloudera.org:8080/24283 )
Change subject: IMPALA-14962: Query Profile Parser and Section Retrieval Interface ...................................................................... Patch Set 6: (9 comments) http://gerrit.cloudera.org:8080/#/c/24283/5/be/src/service/query-profile-parsing-tools.h File be/src/service/query-profile-parsing-tools.h: http://gerrit.cloudera.org:8080/#/c/24283/5/be/src/service/query-profile-parsing-tools.h@33 PS5, Line 33: > Could you please add a detailed comment at the top of this class or maybe t Done http://gerrit.cloudera.org:8080/#/c/24283/5/be/src/service/query-profile-parsing-tools.h@64 PS5, Line 64: /// - Accessor methods return JSON values (mostly objects or arrays depending on method). : /// - Methods that cannot find a section generally return an empty JSON o > This can be a const function Done http://gerrit.cloudera.org:8080/#/c/24283/5/be/src/service/query-profile-parsing-tools.cc File be/src/service/query-profile-parsing-tools.cc: http://gerrit.cloudera.org:8080/#/c/24283/5/be/src/service/query-profile-parsing-tools.cc@119 PS5, Line 119: static string_view TrimWhiteSpace(str > It looks we should add a DCHCK for this, but it seems fine we have this her Done http://gerrit.cloudera.org:8080/#/c/24283/5/be/src/service/query-profile-parsing-tools.cc@224 PS5, Line 224: root_profile = &source_json; > Can we use the string_view here? Done http://gerrit.cloudera.org:8080/#/c/24283/5/be/src/service/query-profile-parsing-tools.cc@255 PS5, Line 255: selected_query_type_query = true; : } : i > Maybe we can optimize the code like this, reduce the map search cost and st Done http://gerrit.cloudera.org:8080/#/c/24283/5/be/src/service/query-profile-parsing-tools.cc@412 PS5, Line 412: if (key.size() >= prefix.size() + norm > Can we use the string_view here? Done http://gerrit.cloudera.org:8080/#/c/24283/5/be/src/service/query-profile-parsing-tools.cc@459 PS5, Line 459: ment_holder = GetFragment(fragment_id, t > Can we use the string_view here? Done http://gerrit.cloudera.org:8080/#/c/24283/5/be/src/service/query-profile-parsing-tools.cc@752 PS5, Line 752: || !summary->HasMember(EVENT_SEQUENCE > This function should be "const void QueryProfileToolAccessor::GatherItems" Done http://gerrit.cloudera.org:8080/#/c/24283/5/be/src/service/query-profile-parsing-tools.cc@756 PS5, Line 756: nst Value& sequences = (*summary)[EVENT_ > Can we use the string_view here? Done -- To view, visit http://gerrit.cloudera.org:8080/24283 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d9c9e735aa089bb243b07af421553002a465a88 Gerrit-Change-Number: 24283 Gerrit-PatchSet: 6 Gerrit-Owner: Gokul Kolady <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Gokul Kolady <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Sat, 16 May 2026 20:16:10 +0000 Gerrit-HasComments: Yes
