Jason Fehr has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24284 )

Change subject: IMPALA-14963: Query Profile AI Analysis Tool
......................................................................


Patch Set 7:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/24284/7/be/src/service/query-profile-ai-analysis.cc
File be/src/service/query-profile-ai-analysis.cc:

http://gerrit.cloudera.org:8080/#/c/24284/7/be/src/service/query-profile-ai-analysis.cc@530
PS7, Line 530:           string retry_message;
             :           retry_message.reserve(
             :               strlen("The previous tool call for '' was ignored 
because tool_call id was "
             :                      "missing. Retry with a valid tool_call id.")
             :               + tool_name.size());
             :           retry_message.append("The previous tool call for '")
             :               .append(tool_name.data(), tool_name.size())
             :               .append("' was ignored because tool_call id was 
missing. Retry with a valid "
             :                       "tool_call id.");
             :           conversation.emplace_back("user", 
std::move(retry_message), "", "");
Replace this code with a call to strings::Substitute("The previous tool call 
for '$0' was ignored because tool_call id was missing. Retry with a valid 
tool_call id.", tool_name);

Will need to:
#include <gutil/strings/substitute.h>


http://gerrit.cloudera.org:8080/#/c/24284/7/be/src/service/query-profile-ai-analysis.cc@575
PS7, Line 575:   const string profile_text_str(profile_text);
Why is a copy of profile_text being created here instead of the previous way of 
using the profile_text string_view directly?


http://gerrit.cloudera.org:8080/#/c/24284/7/be/src/service/query-profile-ai-analysis.cc@614
PS7, Line 614: string(tool_name), string(tool_args_json)
Why create copies of the tool_name and tools_args_json strings?  Why not use 
the tool_name and tool_args_json string_view directly?



--
To view, visit http://gerrit.cloudera.org:8080/24284
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9376b8c56df75032f51abbb76b3ac6927f9107e
Gerrit-Change-Number: 24284
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: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Yida Wu <[email protected]>
Gerrit-Comment-Date: Mon, 18 May 2026 20:26:59 +0000
Gerrit-HasComments: Yes

Reply via email to