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 7:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/24283/6/be/src/service/query-profile-parsing-tools.h
File be/src/service/query-profile-parsing-tools.h:

http://gerrit.cloudera.org:8080/#/c/24283/6/be/src/service/query-profile-parsing-tools.h@109
PS6, Line 109:
             :  private:
             :   std::string_view GetQueryType(const rapidjson::Value& 
query_obj) const;
             :   const rapidjson::Value* SelectScopedValue(const 
rapidjson::Value& value) const;
             :   const rapidjson::Value* GetScopedExe
> explain each with examples where required. For instance is fragments_ all f
Done


http://gerrit.cloudera.org:8080/#/c/24283/6/be/src/service/query-profile-parsing-tools.cc
File be/src/service/query-profile-parsing-tools.cc:

http://gerrit.cloudera.org:8080/#/c/24283/6/be/src/service/query-profile-parsing-tools.cc@50
PS6, Line 50: static constexpr const char CONTENTS_KEY[] = "contents";
            : static constexpr const char PROFILE_NAME_KEY[] = "profile_name";
            : static constexpr const char CHILD_PROFILES_KEY[] = 
"child_profiles";
            : static constexpr const char INFO_STRINGS_KEY[] = "info_strings";
            : static constexpr const char NUM_CHILDREN_KEY[] = "num_children";
            : static constexpr const char VALUE_KEY[] = "_value";
            : static constexpr const char SUMMARY_KEY[] = "Summary";
            : static constexpr const char QUERY_TYPE_KEY[] = "Query Type";
            : static const string QUERY_TYPE_QUERY = 
PrintValue(TStmtType::QUERY);
            : static constexpr const char EVENT_SEQUENCES_KEY[] = 
"event_sequences";
            : static constexpr const char EVENTS_KEY[] = "events";
            : static constexpr const char EXECUTION_PROFILE_PREFIX[] = 
"Execution Profile";
            : static constexpr const char PER_NODE_PROFILES_KEY[] = "Per Node 
Profiles";
            : static constexpr const char ERROR_KEY[] = "error";
            : static constexpr const char INSTANCE_TOKEN[] = "Instance";
            : static constexpr const char FRAGMENT_ID_KEY[] = "fragment_id";
            : static constexpr const char TOOL_UNKNOWN_PREFIX[] = "Unknow
> These should be const char*. You're anyways converting these to cstring usi
Done


http://gerrit.cloudera.org:8080/#/c/24283/6/be/src/service/query-profile-parsing-tools.cc@410
PS6, Line 410:   for (const auto& fragment : fragments_) {
> Explain what we use scoring for?
Done


http://gerrit.cloudera.org:8080/#/c/24283/6/be/src/service/query-profile-parsing-tools.cc@834
PS6, Line 834:     has_loose_substring_match = true;
> If you search for node "1", this matches "10", "11", "21", etc. — returning
Added score-based matching to prioritize full string matches over substring 
matches


http://gerrit.cloudera.org:8080/#/c/24283/6/be/src/service/query-profile-parsing-tools.cc@855
PS6, Line 855:   DCHECK(best_score != nullptr);
> This allocates + transforms on every recursion level. The pattern should be
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: 7
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: Sun, 17 May 2026 20:22:51 +0000
Gerrit-HasComments: Yes

Reply via email to