Gokul Kolady has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24282 )

Change subject: IMPALA-14961: Query Profile Redaction
......................................................................


Patch Set 8:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc
File be/src/service/query-profile-redaction.cc:

http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@142
PS7, Line 142: // Materializes replacement map entries as string_views and 
sorts them
> Add comment explaining what this function does.
Done


http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@149
PS7, Line 149:     entries.emplace_back(entry.first, entry.second);
> Add a comment explaining why we need to sort the entries.
Done


http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@157
PS7, Line 157:       });
> Add comments explaining what this function does.
Done


http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@167
PS7, Line 167:     if (entry.first.find(token) != string_view::npos) return 
true;
> Add comments explaining what this function does.
Done


http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@191
PS7, Line 191: static Status ApplyAliasMap(const unordered_map<string, string>& 
alias_map,
> We could use vector<pair<string, string_view>> pending_restores as the 'to'
Done


http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@201
PS7, Line 201:
> We can pass 'from' as is (string_view)
Done


http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@202
PS7, Line 202:   // Phase 1: replace original tokens with unique sentinels.
> We can pass 'to' as is (string_view)
Done


http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@302
PS7, Line 302:       values.insert(
             :           values.end(), make_move_iterator(child_values.begin()),
             :           make_move_iterator(child_values.end()));
             :     }
             :     return values;
             :   }
             :   if (!node.IsObject()) return values;
             :
             :   for (auto it = node.MemberBegin(); it != node.MemberEnd(); 
++it) {
             :     if (strcmp(it->name.GetString(), "info_strings") == 0) {
             :       if (!it->value.IsArray()) continue;
             :       for (const auto& entry : it->value.GetArray()) {
             :         const auto [key, value] = ParseInfoStringEntry(entry);
             :         if (key == nullptr || value == nullptr) continue;
             :
> Here is similar to CollectIdentifierContextsFromJsonProfile().
Done


http://gerrit.cloudera.org:8080/#/c/24282/7/be/src/service/query-profile-redaction.cc@418
PS7, Line 418: tring, string>& alias_to_origin
> Trying to understand what the sql_statement input would be like, is it "SEL
sql_statement comes from entry["value"].GetString() on a parsed 
rapidjson::Value, which gives the unescaped string content.

*redacted is the original JSON text blob, so the code calls 
JsonEscapeString(sql_statement) to convert it to the escaped form (\"...\") 
before replace_all.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c5b4911a64888f319f212155df6e08c1800b32
Gerrit-Change-Number: 24282
Gerrit-PatchSet: 8
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: Fri, 15 May 2026 04:44:15 +0000
Gerrit-HasComments: Yes

Reply via email to