Jason Fehr 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 4: (31 comments) http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools-test.cc File be/src/service/query-profile-parsing-tools-test.cc: http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools-test.cc@57 PS3, Line 57: > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.h File be/src/service/query-profile-parsing-tools.h: http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.h@31 PS3, Line 31: typedef rapidjson::Document ParsedProfile; > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.h@34 PS3, Line 34: public: : explicit QueryProfileToolAccessor(ParsedProfile parsed); : rapidjson::Value GetSummary(rapidjson::Document::AllocatorType& alloc) const; : rapidjson::Value GetTimeline(rapidjson::Document::AllocatorType& alloc) const; : rapidjson::Value GetCompilation(rapidjson::Document::AllocatorType& alloc) const; : rapidjson::Value GetExecutionProfile(rapidjson::Document::AllocatorType& alloc) const; : rapidjson::Value GetFragmentsOverview(rapidjson::Document::AllocatorType& alloc) const; : rapidjson::Value GetFragment( : const std::string& fragment_id, rapidjson::Document::AllocatorType& alloc) const; : rapidjson::Value GetFragmentInstances( : > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.h@68 PS3, Line 68: void F > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc File be/src/service/query-profile-parsing-tools.cc: http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@50 PS3, Line 50: static const string PROFILE_NAME_KEY = "profile_name"; > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@55 PS3, Line 55: atic > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@79 PS3, Line 79: :\b[A-Z_]* > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@100 PS3, Line 100: cons > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@109 PS3, Line 109: > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@113 PS3, Line 113: return Value(src, alloc); > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@119 PS3, Line 119: > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@133 PS3, Line 133: static Status ConvertJsonProfile( > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@159 PS3, Line 159: ey, Clo > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@168 PS3, Line 168: return Status("'child_profiles' must be an array"); : } : for (const auto& child : profile[CHILD_PROFILES_KEY.c_str()].GetArray()) { : if (!child.IsObject() || !child.HasMember(PROFILE_NAME_KEY.c_str()) : || !child[PROFILE_NAME_KEY.c_str()].IsString()) { : continue; : } : const char* child_name = child[PROFILE_NAME_KEY.c_str()].GetString(); : Value child_value(rapidjson::kObjectType); : > Removed redundant loop Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@189 PS3, Line 189: if (source_json.HasParseError() || !source_json.IsObject()) { : return Status > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@205 PS3, Line 205: Value converted(rapidjson::kObjectType); : RETURN_IF_ERROR(ConvertJsonProfile(*root_profile, &converted, alloc)); : out->AddMember(Value(profile_name.c_str(), alloc), converted, alloc); : return Status::OK(); : } : : return Status("Query profile JSON must include either 'contents.profile_name' " : > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@219 PS3, Line 219: : int query_type_ > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@234 PS3, Line 234: if (key == query_key_) query_type_query_scope_idx_ = query_type_query_index; > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@266 PS3, Line 266: string bucket = item.first; > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@317 PS3, Line 317: // Returns a condensed list of all fragments with summary metadata. : Value QueryProfileT > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@409 PS3, Line 409: for (auto it = fragment.MemberBegin(); it != fragment.MemberEnd(); ++it) { : const string > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@418 PS3, Line 418: // Returns a matching execution node, optionally scoped to a fragment. : Value QueryProfileToolAccessor::GetNode(const string& node_id, const string& fragment_id, : Document::AllocatorType& alloc) const { : const Value* search_data = &parsed_; : Document tmp; : tmp.SetObject(); : Value fragment_holder; : if (!fragment_id.empty()) { : > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@540 PS3, Line 540: while (getline(ss, table, ',')) { : string trimmed = table; : StripWhiteSpace(&trimmed); : if (!trimmed.empty()) arr.PushBack(Value(trimmed.c_str(), alloc), alloc); : } : > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@547 PS3, Line 547: : // Returns reservation and estimate fields from summary/plan text. : Value QueryProfileToolAccessor::GetResourceEstimates( : Document::AllocatorType& alloc) const { : Value obj(rapidjson::kObjectType); : const Value* summary = GetSummaryPtr(); : > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@683 PS3, Line 683: > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@705 PS3, Line 705: return out; > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@853 PS3, Line 853: all > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@906 PS3, Line 906: if (tool_name == "get_resource_estimates") return profile.GetResourceEstimates(alloc); > Done Done http://gerrit.cloudera.org:8080/#/c/24283/3/be/src/service/query-profile-parsing-tools.cc@944 PS3, Line 944: return Status("tool_args_json must be a valid JSON object"); : } : } : : Document out_doc; : out_doc.SetObject(); : > This function appears to only exist for testing purposes. Move it to query Done http://gerrit.cloudera.org:8080/#/c/24283/4/be/src/service/query-profile-parsing-tools.cc File be/src/service/query-profile-parsing-tools.cc: http://gerrit.cloudera.org:8080/#/c/24283/4/be/src/service/query-profile-parsing-tools.cc@272 PS4, Line 272: best_by_fragment_id[bucket] = item; Do a move(item) here to avoid duplicating strings. http://gerrit.cloudera.org:8080/#/c/24283/4/be/src/service/query-profile-parsing-tools.cc@276 PS4, Line 276: entry.second Do a move(entry.second) here to avoid duplicating strings. -- 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: 4 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: Thu, 14 May 2026 22:23:07 +0000 Gerrit-HasComments: Yes
